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

Improve RC workflow: Enhanced validation and error handling in Makefile #270

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

borland667
Copy link
Contributor

What?

  • Improved the bump-test-version target in the Makefile to ensure proper validation of the provided release candidate (RC) versions.
  • Added logic to fetch the latest version and the latest RC version from PyPI.
  • Implemented checks to validate that the provided RC version is either starting from rc0 if no RC versions exist or incrementing the latest RC version from PyPI.
  • Enhanced error handling and messaging to provide clearer feedback during the version bump process.

Why?

  • To ensure that version increments are consistent and follow the existing versioning scheme on PyPI.
  • To prevent invalid RC versions from being set, which could lead to confusion and potential issues during the release process.
  • To automate the version bump process with proper validation, reducing manual errors and improving the release workflow.
  • To provide clear and informative messages to developers when using the Makefile, improving the overall development experience.

References

Before release

Review the checklist here

@borland667 borland667 self-assigned this May 28, 2024
@borland667 borland667 changed the title Improve test release Improve RC Workflow: Enhanced Validation and Error Handling in Makefile May 28, 2024
@coveralls
Copy link
Collaborator

coveralls commented May 28, 2024

Pull Request Test Coverage Report for Build 9271628389

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 69.031%

Totals Coverage Status
Change from base Build 8847843590: 0.0%
Covered Lines: 2018
Relevant Lines: 2812

💛 - Coveralls

@borland667 borland667 changed the title Improve RC Workflow: Enhanced Validation and Error Handling in Makefile Improve RC workflow: Enhanced validation and error handling in Makefile May 28, 2024
Copy link
Contributor

@angelofenoglio angelofenoglio left a comment

Choose a reason for hiding this comment

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

Maybe we can document the version calculation logic so that we can all easily review it, makefile syntax makes it a little hard to follow, what do you think?

Comment on lines +69 to +74
@echo "[INFO] Fetching the latest version from PyPI."
$(eval LATEST_VERSION=$(shell curl -sL "https://pypi.org/pypi/leverage/json" | jq -r ".releases | keys[]" | $(SORT) -V | tail -n 1))
@echo "[INFO] Latest version fetched from PyPI: $(LATEST_VERSION)"

# Get the latest RC version from PyPI if it exists
$(eval LATEST_RC_VERSION=$(shell curl -sL "https://pypi.org/pypi/leverage/json" | jq -r ".releases | keys[]" | grep "$(LATEST_VERSION)-rc" | $(SORT) -V | tail -n 1))
Copy link
Contributor

Choose a reason for hiding this comment

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

At the time of writing this comment:

$ curl -sL "https://pypi.org/pypi/leverage/json" | jq -r '.releases | keys[]' | sort -V | tail -n 2
1.12.3
1.12.3rc1

And so, after running these two statements

LATEST_VERSION=1.12.3rc1
LATEST_RC_VERSION=""

since rcs currently do not use the dash separator. But if we ignore this, both variables would hold the same value.
Suppose we don't have 1.12.3rc1 as a released version, we would end up with something like

LATEST_VERSION=1.12.3
LATEST_RC_VERSION=1.12.2rc3

which would create various different issues down the line like attempting to release 1.12.3rc4 without the previous 3 existing (following the logic in lines 77 to 83).

Couldn't we rely only on LATEST_VERSION?

Comment on lines +101 to +105
if [ "$(PROVIDED_RC_VERSION)" -le "$(shell echo $(LATEST_RC_VERSION) | sed 's/.*-rc//')" ]; then \
echo "[INFO] Provided RC version is valid."; \
else \
echo "[ERROR] Provided RC version is invalid as it does not follow the latest RC version from PyPI. Exiting..."; \
exit 1; \
Copy link
Contributor

Choose a reason for hiding this comment

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

By accepting PROVIDED_RC_VERSIONs that are lesser or equal that the current latest rc version we will most likely clash with an already existing release, wouldn't we? also we wouldn't be able to progress in the rcs version if I'm not mistaken.
I think rcs should be always incrementing in version number since they are for testing purposes, what do you think about it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants