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

configuration section and download cache docs #1544

Merged

Conversation

memsharded
Copy link
Member

@memsharded memsharded requested a review from danimtb February 4, 2020 12:05
@memsharded memsharded added this to the 1.22 milestone Feb 4, 2020
@memsharded memsharded requested a review from jgsogo February 4, 2020 12:07
Copy link
Member

@danimtb danimtb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes and some questions that maybe help to detail the explanation a little bit more:

  • Cache the download cache path to reuse in different builds?
  • Point it to a folder inside .conan folder?

Other questions:

  • Does this have something to do with the short_paths feature on windows?
  • Are files copied to the Conan cache as usual or are these paths used instead?

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expected this to be a short section, maybe one-two paragraphs: it is a cache, it is an optimization, it should be transparent for the user to use, no need to know about the internals. For me, it would be enough to have a brief explanation of the purpose, how to activate/deactivate it (and remove), and the warnings

..but let's hear the feedback from other reviewers

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the team is ok, great then. But I will remove some implementation details and move the CI comments together with other comments about CI.

@memsharded
Copy link
Member Author

Done, removed some implementation details, added note about proxies, and merged some suggestions.

@memsharded memsharded merged commit c902069 into conan-io:develop Feb 4, 2020
@memsharded memsharded deleted the feature/configuration_download_cache branch February 4, 2020 21:47
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.

4 participants