GatorEducator / execexam

:rocket: ExecExam runs executable examinations that assess Python programming skills
https://pypi.org/project/execexam/
1 stars 3 forks source link

feat: Automatic PyPI Publishing using Tags #18

Open PCain02 opened 1 week ago

PCain02 commented 1 week ago

1. Make sure the title is descriptive of what the PR includes. Don't mention issue names/numbers; save that for the description.

2. List the names of those who contributed to the project. @PCain02

3. Link the issue the pull request is meant to fix/resolve. Closes https://github.com/GatorEducator/execexam/issues/2

4. Describe the contents and goal of the pull request. This PR adds the ability to publish to PyPI automatically when using tags. publish.yml and test-publish.yml were the main targets of the PR. The lowercase t tag will be used for test publishing and the lowercase v tag is for real publishing. They should look like t0.3.1 and v0.3.1

I also deleted the empty test files and the "Rebekah Test" print statement so it would pass the lining checks and make it so we can merge into the main execexam.

5. What operating systems has this been tested on? How were these tests conducted? Windows 10 but I would like it reviewed on MacOS and Linux before pushing. The tests were conducted by running the following commands (replace the version number with the actual version next in line):

poetry version 0.3.1 git add pyproject.toml git commit -m "Chore: Bump version to 0.3.1" git tag t0.3.1 git push origin t0.3.1

You will know it worked if Test PyPI has updated the version number

Note: we won't be able to use the v tag until this is in the actual execexam and Professor Kapfhammer's secret key has been added.

6. Add all labels that apply. (e.g., documentation, ready-for-review) Infrastructure

7. Will coverge be maintained/increased? This does not affect coverage.

8. Include a code block and/or screenshots displaying the functionality of your feature, if applicable/possible.

PS C:\Users\Palla\Documents\GitHub\execexam> poetry version 0.3.30
Bumping version from 0.3.30 to 0.3.30
PS C:\Users\Palla\Documents\GitHub\execexam> git add .
PS C:\Users\Palla\Documents\GitHub\execexam> git commit -m 'chore: bump version to 0.3.30 test'
PS C:\Users\Palla\Documents\GitHub\execexam> git tag t0.3.30      
PS C:\Users\Palla\Documents\GitHub\execexam> git push origin t0.3.30
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
To https://github.com/PCain02/execexam.git
* [new tag]         t0.3.30 -> t0.3.30

9. Other Notes

The publish.yml file could not be tested without having Professor Kapfhammer's secret token for PyPI and without bumping version numbers. Testing the publish.yml should be discussed with Professor Kapfhammer before bumping the versions. The test publish feature can be done without fear because it is only bumping the version on TestPyPI. The test publish feature was previously tested by Linux, MacOS, and Windows users.

PCain02 commented 6 days ago

Is this ready for review and merging, or does it need tests to properly update the project? Based on the description, I'm inclined to think the latter, in which case I recommend marking this as a draft and putting out a call for others to compete the necessary tests.

Personally disinclined to approve until it is determined that the publish commands work on other OSs, though if those were confirmed, I would approve.

Another note: in the description, you remark on needing a secret from Professor Kapfhammer, so is this complete or not? I recommend clarifying that part of the description either through editing the post or adding a comment.

Thank you!

Hello! So the test-publish.py commands have all been tested and do work as intended. However, the publish file commands can not be tested until Professor Kapfhammer adds his secret Token for PyPI to the repo even more he may not want the publish.py file to be tested directly until we are ready to bump the version number. It really depends on what Professor Kapfhammer is comfortable with. The only difference though between the two files is the v tag versus the t tag so since the test-publish file worked properly with the different operating systems the hope is that the publish file will do the same and we can test that when we are ready for a version bump. Hope this helps!

gkapfham commented 6 days ago

Hi @PCain02 please note that we still need two pending reviews for this PR to be merged. @boulais01 once this is approved, perhaps it is best for me to coordinate with both you and @PCain02 when it comes to merging as I think that it probably needs keys being set.

AlishChhetri commented 5 days ago

Based on the file changes and the multiple confirmations of this feature working as intended, I am willing to approve this PR. It looks great to me! @PCain02 I appreciate the thorough comments and the inclusion of the commands and code blocks demonstrating the functionality. I am still interested in seeing how setting up the PyPI key as a GitHub secret will unfold, but overall, excellent work.

gkapfham commented 2 days ago

I am making some small improvements to the publish.yml file. I will also be adding the appropriate keys to the required environment variables.

gkapfham commented 2 days ago

Please note that this PR bumps the semver to a value that would break the release order. We can only merge PRs that bump the semver if we are following the rules for semantic versioning. Since this will not, in and of itself, make a release to PyPI then it cannot bump the semver.

gkapfham commented 2 days ago

Since I cannot (easily) push to the fork that created PR #18 I created a new PR #24.

There are fixes in this new PR that are essential for us to keep in the final PR.

We cannot merge this PR until there is documentation that show exactly how to perform a tagged release and then the right type of commit. While I can see that this documentation is in the PR itself, if must exist in some other form of documentation for this project before we can merge this PR that supports the release to PyPI.

gkapfham commented 2 days ago

@PCain02 and everyone involved in this PR, I suggest that we close it and focus on PR #24.