docs: Rewrite docs page on lock file#5404
Conversation
|
Did a quick proofread and review and already a bunch of stuff came up. Will finalise this over the coming couple days then should be good for review |
| If you commit the lock file in your library project, you will want to also consider the following: | ||
| - **Upgrading the lockfile:** How often do you want to upgrade the lockfile used by your developers? Do you want to do these upgrades in the main repo history? | ||
| - **Custom CI workflow to test against latest versions:** Do you want to have a workflow to test against the latest dependency versions? If so - you likely want to have the following CI workflow on a cron schedule: | ||
| - Remove the `pixi.lock` before running the `setup-pixi` action |
There was a problem hiding this comment.
This would be the same as running pixi update. You can provide setup-pixi with the run-install: false to avoid the initial install.
There was a problem hiding this comment.
I'm happy to just mention one method. Would you prefer I update to along the lines of your comment here?
These are covered now in the other sections.
83db1bf to
b7b81e7
Compare
docs/workspace/lockfile.md
Outdated
| --- | ||
|
|
||
| Libraries have an evolving nature and need to be tested against environments covering a wide range of package versions to ensure compatibility. | ||
| This includes an environment with the latest available versions of packages. | ||
|
|
||
| ### Libraries: Committing the lockfile | ||
|
|
||
| If you commit the lock file in your library project, you will want to also consider the following: | ||
| - **Upgrading the lockfile:** How often do you want to upgrade the lockfile used by your developers? Do you want to do these upgrades in the main repo history? Do you want to manage this lockfile via (e.g.,) [the Renovate Bot](https://docs.renovatebot.com/modules/manager/pixi/) or via a custom CI job? | ||
| - **Custom CI workflow to test against latest versions:** Do you want to have a workflow to test against the latest dependency versions? If so - you likely want to have the following CI workflow on a cron schedule: | ||
| - Remove the `pixi.lock` before running the `setup-pixi` action | ||
| - Run your tests | ||
| - If the tests fail: | ||
| - See how the generated `pixi.lock` differs from that in `main` by using `pixi-diff` and `pixi-diff-to-markdown` | ||
| - Automatically file an issue so that its tracked in the project repo | ||
|
|
||
| You can see how these considerations above have been explored by the following projects: | ||
| - Scipy (being explored - will update with PR link once available. [Issue](https://github.com/scipy/scipy/issues/23637)) | ||
|
|
||
|
|
||
| ### Libraries: Git-ignoring the lockfile | ||
|
|
||
| If you don't commit the lockfile, you end up with a simplified setup where the lockfile is generated separately for all developers, and for CI. | ||
|
|
||
| In CI, you can avoid the need to solve on every workflow run by caching this lockfile so that its shared between CI on the same day by using - for example - the [Parcels-code/pixi-lock](https://github.com/parcels-code/pixi-lock) action. | ||
|
|
||
| This simplified setup forgoes reproducibility between machines. | ||
|
|
||
| In both approaches, the test suite is used to determine whether the library is working as expected. | ||
|
|
||
| --- | ||
|
|
||
| See the following threads for more detailed discussion on this topic: | ||
| - [prefix.dev Discord: Should you commit the lockfile](https://discord.com/channels/1082332781146800168/1462778624212996209) | ||
| - [Scientific Python Discord: lock files for libraries](https://discord.com/channels/786703927705862175/1450619697224487083) | ||
| - https://github.com/prefix-dev/pixi/issues/5325 |
There was a problem hiding this comment.
Rereading this part, I think it doesn't fit the lockfile documentation. It reads as an experience log instead of an explainer on what a user should do.
I personally don't believe we should motivate users to git ignore the lockfile. But we should explain how to deal with a lockfile-less workflow for that "on the edge" experience, that libary developers need while testing.
What I would like to change this into is an explainer in ways a library developer could deal with this.
e.g.:
- Renovate bot
pixi updatebefore tests- Full lockfile deletion before tests
You could add a link to the pixi-lock action to provide an extra workflow.
FYI: We're still planning to work out this issue: #4457
Allowing users to setup specific environments that don't have lockfiles which could improve this workflow further.
There was a problem hiding this comment.
It reads as an experience log instead of an explainer on what a user should do.
Yeah, I think it does as well.
I have added f66ed32 which I think addresses your concerns:
- recommends committing the lock file
- briefly links to
pixi-lockaction - removes links to the discussion threads
What I would like to change this into is an explainer in ways a library developer could deal with this. e.g.:
- Renovate bot
pixi updatebefore tests- Full lockfile deletion before tests
I feel this is handled in the (newly named) "Additional considerations for libraries" section
from @ruben-arts - thanks!
|
Ok - implemented all review edits. Ready for re-review :) |
|
This looks pretty good to me. Though I think is could be a bit more prescriptive, that is: Workflow Option A:
Workflow Option B:
One thought, and maybe this isn't the place for it, but a workflow I like (definitely for applications, but also for libraries) is:
HTH, Thanks for writing this! |
|
@ruben-arts, sorry to bump this, but are you open to merging this? I think I responded to all your review feedback. Regarding #5404 (comment) , I think my edits already accounted for this (either way, I would rather leave that up to a future PR rather than this one as this PR is already a significant improvement on |
|
I'm not suggesting any particular text here, but a note from a recent experience: One good reason to keep the lock file in the VCS is that when something breaks, you have a record of previous environments that work. I just had this experience with an update to the netcdf lib -- in my CI, a new environment was created from the existing requirements, which brought in the new netcdf lib, and then a couple of my tests failed. But in this case, I had no lock file in VCS -- so there was no way to know what had changed, and no simple way to go back to a working environment. As it happens, I had just noticed that the new netcdf lib had been released -- between that and the nature of the error, it was obvious what the issue was, but it might not always be so obvious. I'll be putting lock files in VCS from now on :-) |
ruben-arts
left a comment
There was a problem hiding this comment.
Hey @VeckoTheGecko,
Thanks for the work. I still have a few remarks, could you look at them? Please try to run pixi run docs to be able to checkout how it looks locally. Maybe there still some rendering issues.
Sorry for the very late review. I find reviewing these long oppinionated documents very hard and I put it off to focus on some other items first.
Co-authored-by: Ruben Arts <ruben.arts@pm.me>
Co-authored-by: Ruben Arts <ruben.arts@pm.me>
Co-authored-by: Ruben Arts <ruben.arts@pm.me>
Co-authored-by: Ruben Arts <ruben.arts@pm.me>
Co-authored-by: Ruben Arts <ruben.arts@pm.me>
I don't know how those rendering bugs slipped in - I should have been more careful with the initial PR. I have gone through all the remarks and addressed them, and checked the build fixing some minor additional rendering things (I think its good now)
I completely understand. Thanks for taking the time to review! :)) |
ruben-arts
left a comment
There was a problem hiding this comment.
Thank you, this looks good now!
Description
When working on #5325 I found there was no way to write what I wanted to in the current structure. I also found that this page wasn't well suited to those who might be unfamiliar with a lock file and how that differs from a manifest (which in my experience is a lot of people I know who use conda).
Marking as draft for now as I still want to do the following:
pixi.lockFixes #5325
How Has This Been Tested?
NA
AI Disclosure
No AI used
Checklist: