-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add a migration script #240
Conversation
Signed-off-by: Leandro Lucarella <[email protected]>
Add a step to reset the migration script. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
d717965
to
b0d6ba5
Compare
it directly. | ||
|
||
```sh | ||
curl -sSL https://raw.githubusercontent.com/frequenz-floss/frequenz-repo-config-python/{{ ref_name }}/cookiecutter/migrate.sh \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not call it version
instead of ref_name
? refname seems to be for some script or tool, not for the user guide text? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a macro, it is expanded in the docs, the user never sees it.
To run it, the simplest way is to fetch it from GitHub and run it directly: | ||
|
||
```console | ||
curl -sSL https://raw.githubusercontent.com/frequenz-floss/frequenz-repo-config-python/v0.10.0/cookiecutter/migrate.sh | sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also provide something like python -m frequenz.repo-config.migrate v0.10.0
or so? Might be more accessible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's the end goal, I even created a repo label part:cli
but the cli tool never came because of time issues. I prefer not to start now because I want to split this repo soon ™️ (to smaller libraries, like https://github.com/frequenz-floss/setuptools-betterproto/) and a repository for the cookiecutter templates. The migration tool should definitely live in the cookiecutter template repo, but other commands should live elsewhere (like the version calculation tools used by the CI).
So for now I preferred to go with something very simple, to see how it works and not to build a huge thing that will need to be teared apart soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments, but generally okay. Approving in case you want to move forward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, only some optional cosmetics.
Co-authored-by: daniel-zullo-frequenz <[email protected]> Signed-off-by: Leandro Lucarella <[email protected]>
4511f87
Enabling auto-merge, as the comments were optional and I think they are sort of addressed. |
Oh, it needs a new approval anyway. |
This might not be the most advanced approach, but it should be a good starting point to greatly improve the UX of the upgrade process.