detect if API breaks on PR

[mikemhenry]

Developers certificate of origin

• I certify that this contribution is covered by the MIT License here and the Developer Certificate of Origin at https://developercertificate.org/.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.84%. Comparing base (21b5f43) to head (8bf58db).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #727   +/-   ##
=======================================
  Coverage   92.84%   92.84%           
=======================================
  Files         134      134           
  Lines        9940     9940           
=======================================
  Hits         9229     9229           
  Misses        711      711           

Flag	Coverage Δ
fast-tests	92.84% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mikemhenry]

See https://mkdocstrings.github.io/griffe/checking/#detected-breakages

We get errors, but that is because we are breaking things, after the 1.0 tag, we can use this check to make sure we update the change log with breakage.

For example:
https://github.com/OpenFreeEnergy/openfe/actions/runs/7925490023/job/21638754579?pr=727#step:4:65

[IAlibay]

This is really cool @mikemhenry - I personally like it.

[IAlibay]

I can't remember what side effect this has anymore - does it mean that we get a green tick even if it fails? If so, we should make it loud and not a merge requirement imho. That way we can have the red tick and know that it was API breaking?

[mikemhenry]

Okay counteroffer, how about if the step fails (which indicates API breakage) a comment is added to the PR saying "This PR breaks API, be sure to add a note in the change log" I was thinking it would be annoying to have a red check if API breaks and you knew it was going to. That way the check will always be green, but it tells us in a more loud way then having to read a CI log file every time.

[IAlibay]

To do that you'll need to trigger a separate workflow iirc. That can be a bit messy / time consuming. Could this be a follow-up PR?

[mikemhenry]

I don't think it needs to trigger a separate workflow. I just added a bit to do that, it worked in 192b5e9 now I just need to add an API break to see if it does what we expect

[IAlibay]

@mikemhenry is it not that anything that needs secrets (like comment posting) will fail for PRs coming from non-org members unless it's in a pull_request_target workflow type?

[atravitz]

I like this! @mikemhenry could you just address this comment before merge?

[github-actions]

🚨 API breaking changes detected!. 🚨

[mikemhenry]

cool, bot posted the comment and the check stayed green, and this is what the output looks like on the action

We can refine the bot to update the comment/or do more, but it works!