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

Add USERNAME and EMAIL env variables #75

Closed
wants to merge 5 commits into from
Closed

Add USERNAME and EMAIL env variables #75

wants to merge 5 commits into from

Conversation

ozaner
Copy link

@ozaner ozaner commented Jul 29, 2022

Added support for a custom username and email to be used in the autogenerated commit. Just set USERNAME and EMAIL respectively in the env variables.

Context

The autogenerated commit this action produces always uses the username and email of the pusher. This commit adds support for changing these variables using the USERNAME and EMAIL environment variables. My particular use case was to mark these autogenerated commits as being created by the github-actions bot:

- name: Commit to Output
  uses: s0/git-publish-subdir-action@develop
  env:
    REPO: self
    BRANCH: output
    FOLDER: outputDir
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
    USERNAME: "github-actions[bot]"
    EMAIL: "github-actions[bot]@users.noreply.github.com"

Checklist

Added support for a custom username and email to be used in the autogenerated commit. Just set `USERNAME` and `EMAIL` respectively in the env variables.
@s0
Copy link
Owner

s0 commented Jul 31, 2022

Hiya, thanks for the contribution, this is a nice addition.

Before we can get this merged in though, we need to add a few things to this PR:

  • Update the README with instructions on how to use these parameters.
  • Regenerate the action code (run npm run build from action/).
  • Add unit-tests that cover the use of these parameters.

I can help you with any of the above or do it myself, but happy to let you have a go if you'd prefer to do it?

@ozaner
Copy link
Author

ozaner commented Aug 2, 2022

I've updated the readme, regenerated the action code, and added a unit test. Not too familiar with typescript (or even javascript) so I might be missing something.

@promise
Copy link
Contributor

promise commented Aug 22, 2022

@s0 bump

Copy link
Owner

@s0 s0 left a comment

Choose a reason for hiding this comment

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

Hi! Thanks so much for this contribution, and for the docs and tests too, and sorry for the delayed review. I have some comments regarding naming, but other than this I think it's good to go! Thanks!

action/src/index.ts Outdated Show resolved Hide resolved
action/src/index.ts Outdated Show resolved Hide resolved
@s0
Copy link
Owner

s0 commented Aug 23, 2022

(also sorry for not including these change requests in the first review, I had some time to think about the naming and only remembered after I saw it in my notifications again)

- The `USERNAME` and `EMAIL` env variables have been renamed `COMMIT_NAME` and  `COMMIT_EMAIL` respectively.
- Updated readme to reflect this.
- Fixed the file name of the username-email spec.
@ozaner
Copy link
Author

ozaner commented Aug 25, 2022

The USERNAME and EMAIL env variables have been renamed, and the readme updated.

Copy link
Owner

@s0 s0 left a comment

Choose a reason for hiding this comment

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

LGTM

@s0 s0 enabled auto-merge August 25, 2022 13:18
@ozaner ozaner closed this by deleting the head repository Sep 27, 2022
@promise promise mentioned this pull request Oct 3, 2022
1 task
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.

3 participants