Skip to content

Conversation

@hackardoX
Copy link

Description

This PR add support for the signoff options supported by the Peter Evans's create-pull-request action

Checklist
  • Tested functionality against a test repository (see "How to test changes")
  • Added or updated relevant documentation (leave unchecked if not applicable)

@hackardoX hackardoX requested a review from cole-h July 9, 2025 21:01
@hackardoX
Copy link
Author

@cole-h Thank you for the review and for spotting the error in the readme 😄 Can you approve it now?

Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

I don't think this will do what you expect:

https://github.com/cole-h/update-flake-lock-test/blob/e3240e49453c6284996687e7e8d0a8b493b63d5e/.github/workflows/update.yml#L21
cole-h/update-flake-lock-test#42

Notice how there's no signoff line in this PR's commit:

image

This is likely because Nix itself is making the commit. If you want a signoff line, you'll need to manually change the commit message after it's been made, it seems.

@hackardoX
Copy link
Author

Oh I see, I missed that!. What if we do remove the --commit-lock-file option from nix.ts, and we let the create-pull-request action take care of that?

@hackardoX hackardoX force-pushed the feat/signoff branch 3 times, most recently from 7ca7dcd to 8315a03 Compare July 9, 2025 22:31
@hackardoX
Copy link
Author

I played a bit with the action, and I finally make it work using create-pull-request action instead of using nix:
image

https://github.com/andrea11/update-flake-lock-test/pull/3/commits

What do you think about this approach?

@hackardoX hackardoX requested a review from cole-h July 9, 2025 22:55
@hackardoX hackardoX force-pushed the feat/signoff branch 10 times, most recently from 8d83460 to fc9263c Compare July 11, 2025 12:53
@hackardoX
Copy link
Author

hackardoX commented Jul 11, 2025

Hi again @cole-h, I noticed that with my previous changes the flake update output went lost. I updated once more, to get something similar to what we had before:
Before:
image
After:
image

The output is not exactly the same as before, because I am using the output from nix flake update command rather and then delegate the commit to the github action create-pull-request.

Let me know if you consider this blocking

path: pr_body.template
contents: ${{ inputs.pr-body }}
env: {}
- name: Set additional env variables (GIT_COMMIT_MESSAGE)
Copy link
Author

Choose a reason for hiding this comment

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

This is not needed anymore: the variable is set directly inside the index.ts as FLAKE_UPDATE_OUTPUT

const execOptions: actionsExec.ExecOptions = {
cwd: this.pathToFlakeDir !== null ? this.pathToFlakeDir : undefined,
ignoreReturnCode: true,
outStream: new Writable({
Copy link
Author

@hackardoX hackardoX Jul 11, 2025

Choose a reason for hiding this comment

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

instead of this, I tried to use:

const {exitCode, stdout} = await actionsExec.getExecOutput("nix", nixCommandArgs, execOptions);

but the stdout was empty

actionsCore.setFailed(`non-zero exit code of ${exitCode} detected`);
} else {
actionsCore.info(`flake.lock file was successfully updated`);
if (output.length > COMMIT_MESSAGE_MAX_LENGTH) {
Copy link
Author

Choose a reason for hiding this comment

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

This is a safety net in the case the flake output is too huge (it can happen if we have plenty of outdated flakes as dependencies)

Signed-off-by: andrea11 <[email protected]>
@hackardoX
Copy link
Author

@cole-h Any news on this PR? If it is too "complicated" or it has too many changes, let me know. I wanted to tackle this one before #200

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.

2 participants