Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default remappings_version to false #176

Open
marktoda opened this issue Sep 9, 2024 · 3 comments
Open

Default remappings_version to false #176

marktoda opened this issue Sep 9, 2024 · 3 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@marktoda
Copy link

marktoda commented Sep 9, 2024

IMO the remappings version should be defaulted to false. It's quite annoying to have to update every import location on upgrading version, and benefit is unclear to me

@marktoda
Copy link
Author

marktoda commented Sep 9, 2024

Similarly why not make recursive_deps default to true and remappings_regenerate default to true?

IMO defaults should cover the average/basic case, and custom configs for power users who want more complex behavior. I feel that auto-generating simple mappings is the most likely case

@beeb
Copy link
Collaborator

beeb commented Sep 10, 2024

The next version (0.4.0) will default to include a version requirements string in the remapping name, as opposed to an exact version. As such, you will be able to specify @openzeppelin-contracts~5 and that will be the default remapped path (@openzeppelin-contracts-5/=dependencies/...). This means that you will then be able to keep the same imports as long as you don't switch to the v6 of this package. You can even be more granular like @openzeppelin-contracts~^5.0.2. Updates will not affect your existing remappings (the left part of the equal) as long as no major version change has occurred (which must then be updated manually by changing the config file or using the soldeer install command again with a new version specifier).

To expand on this, it's important to include some kind of version info in the remapped path so that if one of your dependencies uses a different major version (e.g. openzeppelin contracts v4.x), both package versions can co-exist and use a different import path to avoid resolution conflicts.

For remappings_regenerate, we want to keep it default to false because that means users can easily customize their remappings without them being overwritten, so it makes sense to be the default. It should have no impact on users who do not customize remappings.

Regarding the recursive install, I think we might consider a change in defaults once we are sure it matches the use-case of everyone and there are no major drawbacks to using it.

@mario-eth mario-eth added the enhancement New feature or request label Sep 20, 2024
@beeb
Copy link
Collaborator

beeb commented Nov 13, 2024

Hey @marktoda , is my answer satisfactory or do you still think the default should change? Please also check this section of the new usage guide for an explanation.

@beeb beeb added the question Further information is requested label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants