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

Move install responsibilities from Resolver to Package #426

Merged
merged 4 commits into from
Aug 4, 2020

Conversation

waj
Copy link
Member

@waj waj commented Jul 31, 2020

This is the first part of a refactor I'm working on to better separate responsibilities between internal entities in Shards. I split these changes in two so it can be reviewed more easily. With this PR the Resolver still represents a source to obtain different versions of a shard, but the installation is delegated completely to the Package class.

This will make more sense with the second part, where locks and installed packages (Shards.info.installed) will return instances of Package instead of Dependency. In other words, finally the Dependency will represent just the intention with some more or less precise definition of which version is desired, while a Package represents a specific version that must be or is already installed.

There is a small change in behaviour after this refactor, but I think in the good direction. Now the only source of truth to determine if a shard is already installed is the .shard.info file. So the contents of the shard could be manually changed, or the symlink created for a path resolver modified but Shards still would assume the dependency is already there and nothing must be done to fix it. Only in the case of a complete removal of the directory or symlink it will reinstall the dependency ignoring what was recorded in the info file.

@waj
Copy link
Member Author

waj commented Aug 3, 2020

I just changed Package#spec so it reads the shard.yml from filesystem when the package is installed. This brings back functionality similar to Resolver#installed_spec. Otherwise, when running spec check or spec list it would "fetch" the source repositories.

@bcardiff bcardiff added this to the v0.12.0 milestone Aug 3, 2020
src/commands/list.cr Outdated Show resolved Hide resolved
@waj waj merged commit b96082c into crystal-lang:master Aug 4, 2020
@waj waj deleted the rework/resolver branch August 4, 2020 13:41
taylor pushed a commit to vulk/shards that referenced this pull request Aug 11, 2020
f-fr pushed a commit to f-fr/shards that referenced this pull request Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants