Comments (13)
DryIoc.Internal makes all public types internal including the types from FEC and ImTools.
Thinking about this Today and I see two ideal choices:
- Make FEC and ImTools sources in DryIoc into the implementation detail - internalize the public types, optionally strip the unused code.
Pros:
- No confusion with standalone FEC and IT
- Still no external deps to manage
- Easy to tune for DryIoc needs
- Less code and dll size if the stripping is in place
Cons:
- Breaking DryIoc major version
- More management from my side - harder to incorp the fixes (though it maybe less fixes because of stripped code)
- Make FEC and IT the explicit dll deps.
Pros:
- Familiar dll story
- Easy to incorp new fixes
- Non-breaking?
Cons:
- Harder to tune for DryIoc
- Dll deps hell
- Not sure how to proceed with the source packages - breakes here?
@detoxhby What are your thoughts?
from imtools.
Thanks for the ideas, I will think about the options how to solves this in a balanced manner!
In the meantime, what about my original idea?
In the external libs (FEC, IT), introducing a preprocessor #if
directive based macro that would include .Embedded
after every single namespace. Could be able to manage the refactor with regex-based replace.
For example:
namespace ImTools
#if IMTOOLS_EMBEDDED
.Embedded
#endif
{
...
Pros:
- no maintenance costs: would be able to include the unmodified source into DryIoc
- insignificant refactor needs:
- relatively easy find in path + replace
using ImTools
byusing ImTools.Embedded
- set
IMTOOLS_EMBEDDED
symbol for compilation
- relatively easy find in path + replace
- no change in distribution options: people still be able to include the sources directly into their code without any change or the need to set a precompile symbol (becase the default behavior would produce the corrrect namespace)
⚠️ no external changes: DryIoc users would've never used the extra libs directly from DryIoc, so would not break legal code
Cons:
- ❗️ external changes: would unintentionally break working code just by the fact the mistaken references are would no longer be available
No easy way to deal with the con other than jumping at least a minor version.
I know the semver would make it necessary to jump a major, but I don't think it is a direct API change, it is just an incorporating dependency change which is outside the libraries' functionality.
Another option is to introduce a new NuGet package (like DryIoc.Embed
) which would behave this way without making the useful types internal.
from imtools.
I have some serious deadlines currently for the next few weeks
Maybe I have time to look next week.
I am thinking about minor versions for FEC and IT, and probably major for DryIoc. I'd like to have an oportunity and mark some things Obsolete in DI. Given that prev release of DI 4.1 was stretching a line of minor version - now is time for fair and square major change.
from imtools.
✔️ Alright, this is not a rush, take the time necessary for a correct major version change!
Feel free to contact me if you need more information or a second-eye with ideas or realization details.
from imtools.
implemented in DryIoc v5
from imtools.
Thanks for the open issue.
It may be a real PITA actually.
What is your setup?
- DryIoc source or dll package?
- ImTools source or dll package?
- Are they referenced from the different package
- Did you know about DryIoc.Internal package?
from imtools.
DryIoc source or dll package?
ImTools source or dll package?
DLL for both packages.
Are they referenced from the different package
This is the problem: cannot reference it as the compiler gets confused as both assemblies contains the same namespace.
Did you know about DryIoc.Internal package?
I did not, but checking out the nuspec file it will still include all the external lib sources and would cause the conflict.
Please correct me if I am mistaken.
To be complete, the same happens with FastExpressionCompiler as well. In that case, we had to change the namespace locally to be able to use it separately from DryIoc. Would be the solution for ImTools as well, but I don't wanna go this way because source inclusion...
- adds unnecessary extra time for compilation, and
- confuses code quality tools (like SonarQube) that will throw numerous warnings, and would impair the code duplication, reliability and other indexes, thus altering the real state of the project's source code ( this is what happened with FEC, not good )
from imtools.
You mean IMTOOLS_EMBEDDED
will be set in DryIoc packages, right?
Then I think it is a good thing to go when thinking in the smaller steps. Thanks for the idea.
You can submit a PR if you want.
I'll probably will risk to go with the v4.2 - I've planned a more cleanup for the v5... lemmethink
from imtools.
You mean IMTOOLS_EMBEDDED will be set in DryIoc packages, right?
Yes!
You can submit a PR if you want.
I have some serious deadlines currently for the next few weeks, but after that, I would gladly help with this work throughout all the projects (FEC, IT, DryIoc).
How do you want to handle cross-project refences to coordinate this change? (FEC+IT first with minor version change and release THEN DryIoc with these new versions?)
from imtools.
Maybe a crazy thought, but why not make ImTools independent, and simply have Dryloc reference ImTools? That way, if projects reference both, there won't be any problem anymore, as there'll be only one shared library for the ImTools classes.
from imtools.
Maybe a crazy thought, but why not make ImTools independent, and simply have Dryloc reference ImTools?
I've answered to this with the second choice in the discussion above.
Mainly I don't wont to get the same thing as with Json.NET dep.
Plus I still want a freedom in DryIoc to use a specially tuned versions of deps. It is just an impl detail - let's keep it that way.
@detoxhby
I'm thinking about simplier variant to your idea. Just change the namespaces in DryIoc to DryIoc.ImTools
and DryIoc.FastExpressionCompiler
respectevely.
from imtools.
I've answered to this with the second choice in the discussion above.
Apologies, I thought I read the whole discussion, must've missed it ;)
It is just an impl detail - let's keep it that way.
Fair enough 👍
from imtools.
I'm thinking about simplier variant to your idea. Just change the namespaces in DryIoc to DryIoc.ImTools and DryIoc.FastExpressionCompiler respectevely.
That's fine too, but a bit error more prone. If you copy over new code and forget about the namespace, you could easily make a VS quick action include mistake somewhere in DryIoc's source code unintentionally (there is a chance; but "fixing" the sources would discard this case entirely).
ℹ️ to reason about current single-file situation: I think it's fine to go with the easy way and just rewrite the namespace in the target project
(would not recommend though if IM or FEC would contains more than a few namespaces and/or files)
from imtools.
Related Issues (20)
- Try out 2-3-4 tree HOT 4
- Remove ImMap V1 from the benchmarks in readme and add ImMap234
- Add ImHashMap23 on par with ImMap23
- Add ImMap.GetSurePresentValue method
- Replace the ImMap methods with Update delegate parameter with composable Add and Update methods
- Add a builder-like capability to the ImHashMap via BuildFromDifferent methods HOT 2
- Add the ImHashMap creation methods to accept the array of items and immediately build the final form of the map HOT 1
- Implement native ToJson and fromJson for maps HOT 1
- the Im(Hash)Map.Entry methods should return the entry type but now return the map type HOT 1
- Document Im(Hash)Map implementation quirks and tricks for the future presentation
- Try struct instead of array for PartitionedImMap HOT 1
- Add output of the ImHashMap as mermaid diagram HOT 1
- Merge the ImMap and ImHashMap implementations
- Optimize Enumerable for the PartitionedHashMap
- Reduce ImHashMap memory allocations keeping the speed
- Add AddSureNotPresent methods to compensate for GetSurePresent methods
- Optimize the case of split B2 to B3 when B2 other leaf still accommodate the Entry
- Fast mutable non-concurrent HashMap HOT 1
- Add to the ImHashMap ref state arguments to be propagated into the Update delegates
- ImHashMap<int, SampleClass> vs MemoryOwner<SampleClass> HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from imtools.