Skip to content

Conversation

@DaniBodor
Copy link
Member

@DaniBodor DaniBodor commented Nov 11, 2024

EDIT: I repurposed this PR to do only the tidying up stuff, and the not-closing PRs on release part is now handled by #338

  • 5d075b5: removed one step from the github release workflow that marks PRs as "closed". They should be automatically marked as "merged" in the preceeding job, when the PR is actually merged.
  • 673ccc0 and e4e316e: I auto-formatted some non-python files, mainly removing trailing whitespaces in a bunch of places.
  • 319c860: update cffconvert action. This action already existed when the CFF was faulty and should have picked those issues up but apparently didnt. I now changed it to reflect the example on the website. Walter had added a separate (different) validation step to the github release action, but I figured it doesn't hurt to have one additional one, which runs whenever a change is made to the CFF file.

@DaniBodor DaniBodor requested a review from psomhorst November 11, 2024 12:19
@DaniBodor
Copy link
Member Author

DaniBodor commented Nov 11, 2024

actually, could you hold off on reviewing this for the moment. There's one more thing I'd like to change.

This PR is now ready for review @psomhorst

@DaniBodor DaniBodor changed the title ci: remove close PR step ci: improvements to workflows Nov 12, 2024
@DaniBodor DaniBodor requested review from psomhorst and removed request for psomhorst November 12, 2024 10:40
Copy link
Contributor

@psomhorst psomhorst left a comment

Choose a reason for hiding this comment

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

@DaniBodor Have you tested this? You say the PR will automatically be closed when the PR is merged, but there is no 'merge PR' action, only a 'merge branches' action. I cannot find any documentation on this automatically closing the PR.

See #338 for an alternative way explicitly merge a PR.

If the PR is automatically closed, it should also remove the branch. This means the last action should be superfluous as well.

@DaniBodor
Copy link
Member Author

@DaniBodor Have you tested this? You say the PR will automatically be closed when the PR is merged, but there is no 'merge PR' action, only a 'merge branches' action. I cannot find any documentation on this automatically closing the PR.

That's a good point. I suggest moving forward with either this branch (where the old close line could be changed to a mark as merged line) or with #338, but not both. Either way, we should probably create a fork and test that it works as intended.

Let me know which solution you prefer.

@DaniBodor DaniBodor force-pushed the 335_dont_close_release_dbodor branch from d9f6de7 to 319c860 Compare November 14, 2024 09:55
@DaniBodor DaniBodor requested a review from psomhorst November 14, 2024 09:58
@DaniBodor
Copy link
Member Author

@psomhorst , I've removed the changes to the github release workflow from this PR, so now it is just some minor style changes.

@DaniBodor DaniBodor changed the title ci: improvements to workflows style: formatting non-python files Nov 14, 2024
@DaniBodor DaniBodor changed the base branch from main to develop November 20, 2024 13:52
@DaniBodor DaniBodor merged commit ed54c5b into develop Nov 20, 2024
4 checks passed
@DaniBodor DaniBodor deleted the 335_dont_close_release_dbodor branch November 20, 2024 13:52
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