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

Remove GH action warnings #370

Merged
merged 7 commits into from
Apr 24, 2023
Merged

Conversation

anth-volk
Copy link
Collaborator

@anth-volk anth-volk commented Apr 22, 2023

Fixes #229

Solution: Upgraded EndBug/add-and-commit, used within push.yml, to version 9, as described in this issue solution within the EndBug/add-and-commit repo.

Version 9 does introduce two breaking changes (described here): the action fails if git commit fails, and the action will not attempt to recreate tags when failing to push them. However, it was not possible to test for these two cases.

@MaxGhenis
Copy link
Collaborator

Thanks @anth-volk! Could you please:

  • Replace Fix for: #229 with Fixes #229 in the PR description; this specific syntax will link the PR to the issue so merging it will close the issue
  • Populate changelog_entry.yaml per other recently merged PRs to pass the version check CI test.

@nikhilwoodruff nikhilwoodruff changed the title fix: Remove set-output warnings by upgrading EndBug/add-and-commit to… Remove GH action warnings Apr 22, 2023
@anth-volk
Copy link
Collaborator Author

Ah, my apologies, I should've reviewed the PR requirements before requesting

@anth-volk
Copy link
Collaborator Author

I'm sorry, I'm not understanding the changelog system. I altered changelog_entry.yaml as instructed, adding the details of the relevant changes, but the version check failed, indicating that I hadn't also updated CHANGELOG.md. Is this not automatically generated? I was under the impression that changelog_entry.yaml was automatically pushed to the top of the file, with the relevant change type updating the relevant portion of the semantic version number. Should CHANGELOG.md (and changelog.yaml, for that matter) also be manually updated?

Copy link
Collaborator

@MaxGhenis MaxGhenis left a comment

Choose a reason for hiding this comment

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

You populated the changelog entry correctly, just this minor change and git pull upstream master will do it. The action fails if the branch is behind upstream/master.

changelog_entry.yaml Outdated Show resolved Hide resolved
@anth-volk
Copy link
Collaborator Author

This should now be properly synced with upstream/main and integrate the typographical changes suggested; however, I'm still receiving the same issue when running checks. I'm going to try to dive into this and figure out what it might be.

@anth-volk
Copy link
Collaborator Author

It appears that, when forking, Git does not also copy upstream's tags, so after fetching tags from upsteram master, I should now be able to pass the has-functional-changes GitHub workflow

@nikhilwoodruff
Copy link
Collaborator

Sorry @anth-volk - could you re-file from a branch? I just gave you write access- happy to help if unclear

@nikhilwoodruff
Copy link
Collaborator

Actually... this is mergeable- I've just checked the file change. Thanks Anthony!

@nikhilwoodruff nikhilwoodruff merged commit 9bd07d0 into PolicyEngine:master Apr 24, 2023
@anth-volk
Copy link
Collaborator Author

Great, thanks! Glad to have finally gotten this one in haha. Thanks for all the support, @nikhilwoodruff and @MaxGhenis.

@anth-volk anth-volk deleted the fix/issue229 branch April 24, 2023 18:19
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.

Upgrade to Environment Files
3 participants