Skip to content

Conversation

rvhonorato
Copy link
Member

@rvhonorato rvhonorato commented Jul 17, 2025

This PR removes the bump-version-on-push.yml - while it's a nice implementation it is also outdated and we should use a similar pipeline to the other codes from the group.

This PR re-works the bump-version-on-push.yml, replacing the deprecated bump2version and changing the way the versions are pushed to pypi

@rvhonorato rvhonorato self-assigned this Jul 17, 2025
@rvhonorato rvhonorato requested a review from amjjbonvin July 17, 2025 16:05
@JoaoRodrigues
Copy link
Member

JoaoRodrigues commented Jul 18, 2025

I know I'm a little removed from this project, time constraints and all that, but I'd like to leave a couple comments here.

First, traditionally we've done squash commits to keep the history clean and easy to revert. I see that your last commit did not follow that. I personally consider this to be a bad practice.

Second, I would be against touching a logic that has been working well, stably, without a single issue, for at least 7 years. I get the need to standardize things but this is such a small file and such a straightforward process to understand that I cannot see what benefits would come from changing (radically) this process. Change for the sake of change isn't great.

There are plenty of other things that need work in pdb-tools, CI is definitely not one of them.

@rvhonorato
Copy link
Member Author

I was hoping to get your attention on this one @JoaoRodrigues :) Yep, I myself also prefer to do squash commits and that's how I do in the other repos - in which most of them are configured to be the only possible way to merge. Which is something that is not configured here.

And as for this file, you mentioned it yourself, you are not so involved in this project and I see your credentials are directly linked into this action - something that I consider bad practice.

My motivation to remove this and to re-work the CI is to facilitate the maintenance of this code so it follows in line with the other ones, specially considering its kind of of my job. I would rather not touch this repository at all, so it's definitively not just for the sake of change.

I'm also fine with keeping it - but then please take your time to re-configure the repository to disallow merge commits and/or configure any other setting that follows your practices. I'm going to set you as a reviewer here so you can decide on it.

@rvhonorato rvhonorato requested a review from JoaoRodrigues July 18, 2025 17:55
@JoaoRodrigues
Copy link
Member

I think the documentation here is clear enough regarding the processes we implemented originally. Feel free to change whatever settings and credentials you need, I see you are an admin of the project so you should have the authority to do so.

I was just offering my opinion based on my experience with this project (and others) throughout the years.

@JoaoRodrigues JoaoRodrigues removed their request for review July 18, 2025 18:24
@rvhonorato
Copy link
Member Author

rvhonorato commented Jul 19, 2025

Thanks! I guess we can keep this logic of automatic version bumping. But I'd still suggest a reconfiguration of the repository to enforce the contributing guidelines rather than assume people will follow it and re-work the credentials. Even more so because this project is not under https://pypi.org/user/bonvinlab.

We can re-use this PR to work on that.

Let's follow rule #1 (: @amjjbonvin, would you rather re-configure or keep as is?

@amjjbonvin
Copy link
Member

amjjbonvin commented Jul 19, 2025 via email

@rvhonorato rvhonorato marked this pull request as draft July 21, 2025 08:07
@rvhonorato rvhonorato changed the title remove bump-version-on-push.yml re-work bump-version-on-push.yml Jul 21, 2025
@rvhonorato rvhonorato changed the title re-work bump-version-on-push.yml [SKIP] re-work bump-version-on-push.yml Jul 21, 2025
@rvhonorato rvhonorato marked this pull request as ready for review July 21, 2025 11:25
@rvhonorato
Copy link
Member Author

I added logic in ci.yml to make sure the PR's follow the expected format - they must all contain either [MAJOR/MINOR/PATH/SKIP] in the title, else it will trigger a failure. I updated the triggers so the action is triggered also when people edit the PR.

I changed bump2version in favor if bump-my-version as per recommendation of bump2version repo.

Finally changed the job of publishing a package to PyPI to use Trusted Publisher Management in place of credentials baked in the repository.

Tried to be as conservative as possible in these changes, tested out the bump-my-version in a private repository and it seems to be working fine.

@rvhonorato rvhonorato requested a review from a team July 21, 2025 11:31
@rvhonorato rvhonorato merged commit 3eb93b3 into master Jul 21, 2025
17 checks passed
@rvhonorato rvhonorato deleted the bump-tweaks branch July 21, 2025 15:59
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.

3 participants