std icon indicating copy to clipboard operation
std copied to clipboard
verified publisher icon divnix

feat: make terra plan write diff to github PR

Open oneingan opened this issue 1 year ago • 6 comments

Introduce postDiffToGitHubSnippet for unified diff posting and update kubectl.nix and terra.nix

  • Add postDiffToGitHubSnippet:

    • Created a reusable snippet that handles posting unified diffs to GitHub PR comments using the gh CLI.
    • Manages existing comments by updating them or creates new ones if necessary.

  • Update kubectl.nix:

    • Replaced inline diff posting logic with the new postDiffToGitHubSnippet.

  • Modify terra.nix:

    • Integrated postDiffToGitHubSnippet into the plan command within the wrap function.
    • Exported TF_VAR_fragment and TF_VAR_fragmentRelPath for use in Terraform configurations.
    • Changed the Terraform working directory to $PRJ_ROOT/.cache/${fragmentRelPath}/.tf for better organization.

oneingan avatar Jul 26 '24 09:07 oneingan

Could you take it over and drive it over the finish line, especially with some real testing towards the end?

blaggacao avatar Jul 26 '24 09:07 blaggacao

Currently works!

But i'm still not comfortable with --edit-last option from a monorepo point of view. Different terra plans overwrite between them. I will back with an alternative.

oneingan avatar Jul 30 '24 08:07 oneingan

@oneingan Nice! I'm noticing some movement. Go for it! :handshake:

blaggacao avatar Oct 17 '24 13:10 blaggacao

i make it append multiple plans in a sticky PR comment, also passthru some cell info in TF_VAR. I move the staging area to a different directory than nix and use full fragment path to avoid conflicts.

I thought is ready for review @blaggacao

oneingan avatar Oct 18 '24 01:10 oneingan

image

oneingan avatar Oct 18 '24 01:10 oneingan

I (finally) made sure tests run on main: https://github.com/divnix/std/actions/runs/11517292880 — can you rebase?

blaggacao avatar Oct 25 '24 12:10 blaggacao

Can you give some context on the CI failure?

It might be an accidental, unrelated failure. But I lack a bit of context on that possibility.

If that's the case, I'd have to apologize and fix that first.

blaggacao avatar Nov 09 '24 14:11 blaggacao

Let's ignore the Mac failure, seems pretty much unrelated. I'm glad that linux is green now, thank you!

~~Please tackle any further fixes in follow up PRs, in case it's necessary. Merging. 🚀~~

blaggacao avatar Nov 11 '24 20:11 blaggacao

Hm, the nvfetcher change, adding '''' looks quite unrelated, could you drop that?

blaggacao avatar Nov 11 '24 20:11 blaggacao

sorry i was trying to catch the macos fail, i will revert last commits then

oneingan avatar Nov 11 '24 21:11 oneingan