paulirwin commented on PR #866: URL: https://github.com/apache/lucenenet/pull/866#issuecomment-3246038823
> "Frequently" is a relative term. We review them every time we release and often bump versions and add or remove packages several times between releases. Let's reserve the term "frequently" for when we're releasing more than once a year 😉 Kidding aside, what I meant is that by far the most common scenario between builds is for our dependencies to stay the same, so we should have a high amount of cache hits with this change. > I don't object to caching, but there must be some consideration given both to: > > 1. Whether it changes our development workflow, and if it does, we need documentation so we can upgrade painlessly. > > 2. It must be confirmed to allow us to update dependencies before it is rolled out. Indeed, and these should not be a problem. The lockfile is regenerated automatically when dependencies change. If I take up this PR, I will make sure this is documented in the CONTRIBUTING.md file. > The second issue bit us hard the first time around because the `dependencies.props` file wasn't included in the cache key, which is the main file that we change to bump/add/remove dependencies. Yes, see the docs at the link I sent above. The lockfile will be included in the cache key. Also, regarding the notion of "biting us," the only downside here is a cache miss for a subset packages (likely not all, unless literally all of them change), or a failed build if we enable `--locked-mode` (in which case we just update the lockfile and it will succeed). But that should not be a problem with the lockfile approach when the lockfile hash is included in the cache key. > I like the idea of using a cache key that we don't have to think about, which would have been the case had the `dependencies.props` and all other `.props`, `.targets`, `.msbuildproj` and other MSBuild files been included instead of only including the `.csproj` files. For posterity, including all of those other files should not be needed with the lockfile approach, as the only thing we are caching are the computed dependencies, and those only change if the lockfile changes, and the lockfile will change if the dependencies resolved by the proj/props/targets files change. So including them in the cache key is redundant. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
