From ea4ccd4fa94d4202decd71057abe05e7a8120de6 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 11:30:25 -0700 Subject: [PATCH 01/13] fix issue #1042 and also test openfecli for API breaks --- .github/workflows/change.yaml | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/.github/workflows/change.yaml b/.github/workflows/change.yaml index acc25733a..be329004b 100644 --- a/.github/workflows/change.yaml +++ b/.github/workflows/change.yaml @@ -16,6 +16,8 @@ jobs: with: fetch-depth: 0 + - run: git fetch --depth=1 --tags + - uses: actions/setup-python@v5 with: python-version: "3.12" @@ -26,6 +28,7 @@ jobs: run: | pip install griffe griffe check "openfe" --verbose + griffe check "openfecli" --verbose - name: Post Comment on Failure if: steps.check.outcome == 'failure' @@ -33,9 +36,32 @@ jobs: with: script: | const prNumber = context.payload.pull_request.number; - github.rest.issues.createComment({ + const identifier = ''; + const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; + const stepUrl = `${runUrl}#step:check`; + + // List existing comments + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + }); + + // Delete previous comments from this action + for (const comment of comments) { + if (comment.body.includes(identifier)) { + await github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: comment.id, + }); + } + } + + // Post a new comment + await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber, - body: '🚨 API breaking changes detected! 🚨' + body: `${identifier}\n🚨 API breaking changes detected! 🚨\n[View logs for this step](${stepUrl})` }); From 3a5029d0ab660cc1d83fc22fd9226b1135e540eb Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 11:46:18 -0700 Subject: [PATCH 02/13] just check against main instead of last tag --- .github/workflows/change.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/change.yaml b/.github/workflows/change.yaml index be329004b..396ca91dd 100644 --- a/.github/workflows/change.yaml +++ b/.github/workflows/change.yaml @@ -27,8 +27,8 @@ jobs: id: check run: | pip install griffe - griffe check "openfe" --verbose - griffe check "openfecli" --verbose + griffe check "openfe" --verbose -a origin/main + griffe check "openfecli" --verbose -a origin/main - name: Post Comment on Failure if: steps.check.outcome == 'failure' From 34ad020be5cbcdffb03857629bf1a951cd90999d Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 11:49:41 -0700 Subject: [PATCH 03/13] change to run on PR so I can test the changes --- .github/workflows/change.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/change.yaml b/.github/workflows/change.yaml index 396ca91dd..3bfa4c65a 100644 --- a/.github/workflows/change.yaml +++ b/.github/workflows/change.yaml @@ -1,7 +1,7 @@ name: Check for API breaks on: - pull_request_target: + pull_request: branches: - main From 35c1448e179e1fde4dd3b37ad8d99a7cf47f9e45 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 12:08:34 -0700 Subject: [PATCH 04/13] test cli api change --- openfecli/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfecli/cli.py b/openfecli/cli.py index 014d85f7c..8fba7337b 100644 --- a/openfecli/cli.py +++ b/openfecli/cli.py @@ -38,7 +38,7 @@ def get_installed_plugins(self): @click.command(cls=OpenFECLI, name="openfe", help=_MAIN_HELP, context_settings=CONTEXT_SETTINGS) @click.version_option(version=openfecli.__version__) -@click.option('--log', type=click.Path(exists=True, readable=True), +@click.option('--logs', type=click.Path(exists=True, readable=True), help="logging configuration file") def main(log): # Subcommand runs after this is processed. From 4a0ddf51c9c03c77d8dd69c6a6ebca4c2e753ac4 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 12:23:24 -0700 Subject: [PATCH 05/13] undo change that didn't break api --- openfecli/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfecli/cli.py b/openfecli/cli.py index 8fba7337b..014d85f7c 100644 --- a/openfecli/cli.py +++ b/openfecli/cli.py @@ -38,7 +38,7 @@ def get_installed_plugins(self): @click.command(cls=OpenFECLI, name="openfe", help=_MAIN_HELP, context_settings=CONTEXT_SETTINGS) @click.version_option(version=openfecli.__version__) -@click.option('--logs', type=click.Path(exists=True, readable=True), +@click.option('--log', type=click.Path(exists=True, readable=True), help="logging configuration file") def main(log): # Subcommand runs after this is processed. From 1e898636ef451ea2405d8598a9880ef8cb0196d8 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 12:23:41 -0700 Subject: [PATCH 06/13] change default that will trigger API change --- openfecli/commands/gather.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfecli/commands/gather.py b/openfecli/commands/gather.py index 52a16a2b2..61452b4db 100644 --- a/openfecli/commands/gather.py +++ b/openfecli/commands/gather.py @@ -41,7 +41,7 @@ def _get_column(val:float|int)->int: def format_estimate_uncertainty( est: float, unc: float, - unc_prec: int = 1, + unc_prec: int = 2, ) -> tuple[str, str]: """Truncate raw estimate and uncertainty values to the appropriate uncertainty. From 9bc6bd9b164df9d256f1aee6ace536d1dec2fe23 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 12:36:22 -0700 Subject: [PATCH 07/13] undo api change --- openfecli/commands/gather.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfecli/commands/gather.py b/openfecli/commands/gather.py index 61452b4db..52a16a2b2 100644 --- a/openfecli/commands/gather.py +++ b/openfecli/commands/gather.py @@ -41,7 +41,7 @@ def _get_column(val:float|int)->int: def format_estimate_uncertainty( est: float, unc: float, - unc_prec: int = 2, + unc_prec: int = 1, ) -> tuple[str, str]: """Truncate raw estimate and uncertainty values to the appropriate uncertainty. From 26f7c44334597671ac9ee04d631f42c4b9c79ef1 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 12:48:29 -0700 Subject: [PATCH 08/13] remove comments if api change check passes --- .github/workflows/change.yaml | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/.github/workflows/change.yaml b/.github/workflows/change.yaml index 3bfa4c65a..200aa00bc 100644 --- a/.github/workflows/change.yaml +++ b/.github/workflows/change.yaml @@ -30,8 +30,7 @@ jobs: griffe check "openfe" --verbose -a origin/main griffe check "openfecli" --verbose -a origin/main - - name: Post Comment on Failure - if: steps.check.outcome == 'failure' + - name: Manage PR Comments uses: actions/github-script@v7 with: script: | @@ -58,10 +57,12 @@ jobs: } } - // Post a new comment - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - body: `${identifier}\n🚨 API breaking changes detected! 🚨\n[View logs for this step](${stepUrl})` - }); + // Post a new comment only if the check step failed + if (steps.check.outcome === 'failure') { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: `${identifier}\n🚨 API breaking changes detected! 🚨\n[View logs for this step](${stepUrl})` + }); + } From cb4ebd172e8f3878c716498e89a16971dcf29206 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 14:51:31 -0700 Subject: [PATCH 09/13] fix reference error --- .github/workflows/change.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/change.yaml b/.github/workflows/change.yaml index 200aa00bc..82202d1dc 100644 --- a/.github/workflows/change.yaml +++ b/.github/workflows/change.yaml @@ -39,6 +39,9 @@ jobs: const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; const stepUrl = `${runUrl}#step:check`; + // Determine the outcome of the check step + const checkStepOutcome = '${{ steps.check.outcome }}'; + // List existing comments const { data: comments } = await github.rest.issues.listComments({ owner: context.repo.owner, @@ -58,7 +61,7 @@ jobs: } // Post a new comment only if the check step failed - if (steps.check.outcome === 'failure') { + if (checkStepOutcome === 'failure') { await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, From 0e6eeba213907a7bb91c4a1d6c2c67fe7b3bb2c7 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 15:02:46 -0700 Subject: [PATCH 10/13] break api --- openfecli/commands/gather.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfecli/commands/gather.py b/openfecli/commands/gather.py index 52a16a2b2..61452b4db 100644 --- a/openfecli/commands/gather.py +++ b/openfecli/commands/gather.py @@ -41,7 +41,7 @@ def _get_column(val:float|int)->int: def format_estimate_uncertainty( est: float, unc: float, - unc_prec: int = 1, + unc_prec: int = 2, ) -> tuple[str, str]: """Truncate raw estimate and uncertainty values to the appropriate uncertainty. From fb474de6f51d2f79bf4ca9dbeae44769b14ce772 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 15:23:31 -0700 Subject: [PATCH 11/13] add a message if there is no API break --- .github/workflows/change.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/change.yaml b/.github/workflows/change.yaml index 82202d1dc..2ccf0d535 100644 --- a/.github/workflows/change.yaml +++ b/.github/workflows/change.yaml @@ -68,4 +68,11 @@ jobs: issue_number: prNumber, body: `${identifier}\n🚨 API breaking changes detected! 🚨\n[View logs for this step](${stepUrl})` }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: `${identifier}\nNo API break detected ✅` + }); } From 52f3da4eda08e7c69235a709745cdb296b21bf7d Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 15:25:50 -0700 Subject: [PATCH 12/13] fix api break --- openfecli/commands/gather.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfecli/commands/gather.py b/openfecli/commands/gather.py index 61452b4db..52a16a2b2 100644 --- a/openfecli/commands/gather.py +++ b/openfecli/commands/gather.py @@ -41,7 +41,7 @@ def _get_column(val:float|int)->int: def format_estimate_uncertainty( est: float, unc: float, - unc_prec: int = 2, + unc_prec: int = 1, ) -> tuple[str, str]: """Truncate raw estimate and uncertainty values to the appropriate uncertainty. From 42defe07f4b4ed576355c198fa12f73263bedfe4 Mon Sep 17 00:00:00 2001 From: Mike Henry <11765982+mikemhenry@users.noreply.github.com> Date: Fri, 13 Dec 2024 15:27:19 -0700 Subject: [PATCH 13/13] Switch back to pull request target now that testing is done --- .github/workflows/change.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/change.yaml b/.github/workflows/change.yaml index 2ccf0d535..53a20d314 100644 --- a/.github/workflows/change.yaml +++ b/.github/workflows/change.yaml @@ -1,7 +1,7 @@ name: Check for API breaks on: - pull_request: + pull_request_target: branches: - main