Refactor rc file loading / local registry support + add tests #337
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gasinvein
reviewed
Dec 17, 2022
|
|
||
| class ConfigProvider: | ||
| @property | ||
| def _filename(self) -> str: |
There was a problem hiding this comment.
Is there any reason for the filename to be a property and not a class variable? I can't imagine npm/yarn config file name ever changing.
There was a problem hiding this comment.
It's mostly because then we could use actual abstractproperty handling in the future, but atm it does mean that forgetting to override it gives a clearer error at the call site.
refi64
force-pushed
the
wip/refi64/local-registry
branch
from
85a4382
to
df302eb
Compare
- Use more "proper" package-manager-specific parsing, including what should be a fully-compliant (albiet over-engineered) parser for npm's ini format used in .npmrc. - Moves all config loading into its own set of classes. (As it turns out, this *does* walk recursively, contrary to what I said before, oops!) - Terminology change: rename rcfile -> config, just to match more with how npm describes itself. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
refi64
force-pushed
the
wip/refi64/local-registry
branch
from
df302eb
to
0808f9d
Compare
|
Merge conflicts fixed. |
This adds a test-time dependency on a running Verdaccio instance. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
refi64
force-pushed
the
wip/refi64/local-registry
branch
from
0808f9d
to
b4ade5a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This:
even though that's not really necessary) & yarn rc file parsers w/ tests. (As it turns out, I was wrong when I said .yarnrc is loaded up the directory tree, oops.)