Skip to content

Various fix for your PR #1

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

Merged
merged 7 commits into from
Dec 31, 2023

Conversation

astariul
Copy link

Hey there,

I tested your PR more thoroughly, and found some issues.
At first I thought it would be a quick fix, but actually it took quite some time. Anyway, here is the changes needed to have something mergeable in my repo !


So the main issue was that in the github actions, we are not using link anymore. Instead, we build that link from the version given by the user.

The issue is that in many open-source projects, the tag is named something like v1.2. But if the user enter 1.2, then the link generated is https://github.com/user/project@1.2, which doesn't exist because there is no tag 1.2 (it should be v1.2). Of course it can be in the opposite way as well (user specify v1.2 but the tag is 1.2).

But I still think your change is better (no need to specify a version AND a link, just specify the version). So I clarified it in the github action input description (mentioning we need the tag). Then in the github action, I clean the v1.2 to 1.2 if needed.


I also improved the detection of "pre-release" versions (not only dev, but also rc, a, and b) -> see PEP 440


I also modernized many format into f-strings.


Also in the index file, by default it shows the latest version, even if it's pre-release. I changed it so instead it shows the latest stable version, I think it's better, but what's your opinion ?
Screen Shot 2023-12-30 at 12 22 10 AM


I also removed some remaining anchor tags, to make sure this is compliant to PEP503


And finally, I updated the update_pkgs.py script (by the way, this is such a great idea, now it's super easy to test new changes !!!) to not call subprocesses, but use python directly.

And I fixed the tag names in there, so the links work. Also added a dev version for public-hello in order to have an example in the template (for the pre-release tag) !

@astariul astariul requested a review from pabnas December 30, 2023 08:01
@pabnas pabnas merged commit 3ba584b into pabnas:general_fix_and_update_styles Dec 31, 2023
@astariul astariul deleted the my_branch branch December 31, 2023 03:55
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.

2 participants