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

revise adr to support user-name and user-email #158

Closed
wants to merge 1 commit into from

Conversation

ericsciple
Copy link
Contributor

No description provided.

@@ -179,6 +183,33 @@ A better solution is:

Given a source file path, walk up the directories until the first `.git/config` is found. Check if it matches the self repo (`url = https://github.com/OWNER/REPO`). If not, drop the source file path.


### User name and email
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unclear how this will be impacted by submodules support. probably need to add to local git config for each submodule too

- uses: actions/checkout@v2
with:
user-name: github-actions
user-email: [email protected]

Choose a reason for hiding this comment

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

I am unsure off the top of my head, but is there a way in your actions file to reference the user and email on the commit being built from or the action triggering user? As I mentioned in #13 I would like to be able to amend the commit or make a new one without having different commit ownership.

If not it would be awesome if these were just defaulted to that with these option here to override. Or is there a reason that is not a good idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On push it looks like ${{ github.event.head_commit.author.name }} and ${{ github.event.head_commit.author.email }} can be used.

Haven't looked at values on pull_request. I dumped the context using this:

on: push
jobs:
  build:
    runs-on: [ubuntu-latest]
    steps:
      - run: |
          echo "$GITHUB_CONTEXT"
        env:
          GITHUB_CONTEXT: ${{ toJson(github) }}

I'm hesitant to add any default since it would regress folks who are setting and relying on global config user name. If we make a new major version (v3), then would be an opportunity to break compat.

Choose a reason for hiding this comment

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

Makes sense, as long as there is an easy way to achieve this!

Copy link

@eine eine Feb 17, 2020

Choose a reason for hiding this comment

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

@wesleytodd I believe you can read the context/json and set outputs in the step before using checkout. However, this seems unnecessarily complex. @ericsciple, what do you think about supporting a single user field that expects either username <useremail> or auto? Naturally, auto would imply the behaviour expected by @wesleytodd (and me).

Choose a reason for hiding this comment

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

Does GitHub associate [email protected] with github-actions[bot]? Otherwise, github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> might be a better default as I mentioned in #13.

@wesleytodd Amending a commit does not change the author unless --reset-author or --author=<author> is set. Only the commiter is changed but I think that is fine in most cases since it reflects what is actually happening.

@davidkarlsen davidkarlsen mentioned this pull request May 6, 2020
@davidkarlsen
Copy link

Can this be moved forward? It's too much fuzz today for a simple commit operation back into the repo.

@eine
Copy link

eine commented Aug 20, 2020

@davidkarlsen, although I agree that it would be useful to have this moved forward, I don't think it's "too much fuzz today". It's just 4 rows instead of 2: https://github.com/dbhi/mdpaper/blob/main/.github/workflows/build.yml#L31-L34. Note that, in that example, a new clean repo and branch are created in 3 rows, by reusing the credentials/config setup by this action: https://github.com/dbhi/mdpaper/blob/main/.github/workflows/build.yml#L27-L29. So, overall, this feature would reduce the verbosity from 7 rows to 5.

@chancancode
Copy link

Should this be named "author" instead of "user" to match the Git terminologies, and not to confuse it with the GitHub terminologies? Personally, I think a good default for this would be the @github-actions bot user, which matches the other actions performed by the default token. For example, when you use the default token to create a pull request, it would have been created by the @github-actions user.

IMO, it would be very unexpected if this defaults to the same author that happens to be on the most recent commit, especially when in cron jobs. Having a commit I didn't write myself attributed to me in the Git history is going to be very confusing. For the amend use case mentioned above, what I proposed would have resulted in the original author being preserved, but with the @github-actions bot user as the committer, which seems accurate to me and matches what that Git feature are designed for.

Another option would be to make an API call to fetch the name and the public email of the provided token (which would be @github-actions most of the time, but would Just Work™ if you supply a custom token). There is the additional complexity of what if SSH keys are supplied instead of tokens, but it can be made to work since ssh [email protected] tells you the username.

Personally, I am not sure if that last part is worth it. Attributing the commits to the @github-actions bot user seems perfectly adequate to me. Even when a different token is supplied, most of the time people are going to want to use a deploy token or a custom bot account (as opposed to a token from a personal account), in which case the authorship information is probably just as meaningless as @github-actions. At the end of the day, all you want to know is that the commit was created by some bot, most of the time. If it really matters, it's easy enough to explicitly specify the author name and email using the feature added in this PR, in which case you can change it to whatever you want.

@JojOatXGME
Copy link

JojOatXGME commented Nov 10, 2020

I think a proper default is actually more important then an option to override it. If you want to configure a specific user, you can easily configure Git in a separata step. This means I would actually prefer a single boolean input called configure-user over user-name and user-email.

For me, the main motivation behind letting the action configure the user is to finally answer the question, which email address and username should be used. There seems to be a big variance of email addresses and usernames used for this purpose in different repositories. This include [email protected], 41898282+github-actions[bot]@users.noreply.github.com, [email protected] and [email protected] (all found at a single day while surfing around). Having a default would finally give clear guidance for a reasonable and consistent configuration on GitHub.

@chancancode The terminology of Git is “user” not “author”. See https://git-scm.com/docs/git-config#Documentation/git-config.txt-username. The user can become the author of a commit, though.

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.

6 participants