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

chore: inject RENOVATE_GIT_AUTHOR vars in GitHub Actions #416

Merged

Conversation

huxuan
Copy link
Member

@huxuan huxuan commented Mar 18, 2024

For GitLab, it works natively. Please refer to: https://gitlab.com/huxuan8528/ss-python/-/merge_requests/35/commits. But it is a pity that the random string of the email address on GitLab for bot users is a must have pattern when compared with GitHub.

@huxuan huxuan marked this pull request as draft March 18, 2024 01:36
@huxuan huxuan force-pushed the xuan.hu/inject-renovate-git-author branch from be04d10 to bdbebfd Compare March 18, 2024 02:17
@huxuan huxuan marked this pull request as ready for review March 18, 2024 02:18
@huxuan huxuan requested a review from msclock March 18, 2024 02:19
@msclock
Copy link
Contributor

msclock commented Mar 18, 2024

@huxuan Actually,it is natively true that renovate will resolve the project/group token and set the proper user and email as the commit author of prs. I have renamed and tested a new generated project token for RENOVATE_TOKEN. See https://gitlab.com/serious-scaffold/ss-python-gitlab/-/merge_requests/2 and https://gitlab.com/serious-scaffold/ss-python-gitlab/-/settings/access_tokens.

So on GitLab, we don't need to do any tricks to set the RENOVATE_GIT_AUTHOR.🤣Or just set a dummy git author.

What we should focus is on GitHub, I consider it can just work using the pr #408.

@huxuan
Copy link
Member Author

huxuan commented Mar 18, 2024

@huxuan Actually,it is natively true that renovate will resolve the project/group token and set the proper user and email as the commit author of prs. I have renamed and tested a new generated project token for RENOVATE_TOKEN. See https://gitlab.com/serious-scaffold/ss-python-gitlab/-/merge_requests/2 and https://gitlab.com/serious-scaffold/ss-python-gitlab/-/settings/access_tokens.

So on GitLab, we don't need to do any tricks to set the RENOVATE_GIT_AUTHOR.🤣Or just set a dummy git author.

Yep, the author of the Pull/Merge Request on both GitHub and GitLab should work as expected and almost nothing need to do for GitLab.

But something I want to clairfy is that the key problem is the commit authors, and whether there will be a co-author when squash merge the changes. You can refer to the "Commits" tab of merge requests: https://gitlab.com/serious-scaffold/ss-python-gitlab/-/merge_requests/2/commits and https://gitlab.com/huxuan8528/ss-python/-/merge_requests/35/commits. Without configuration, it will show the dummy author we set and the link is mailto:gitlab@renovatebot.com, and when properly configured, the link will be the profile of the bot user, for example, https://gitlab.com/project_50406584_bot_f3a5d977fd130f8b33e55e803e410fed.

What we should focus is on GitHub, I consider it can just work using the pr #408.

Just as I have mentioned in #400 (comment), it is more like a hack rather than solution and it only solves the cases for GitHub App authentication, not for PAT. I would prefer to keep the whole procedure simple: set a dummy git author while making it injectable. What do you think of?

@msclock
Copy link
Contributor

msclock commented Mar 18, 2024

@huxuan I consider we should avoid generating a ghost author in commits history. How about just aborting the renovate job and exit or skip with a warning job message when no RENOVATE_GIT_AUTHOR setup.

@huxuan
Copy link
Member Author

huxuan commented Mar 18, 2024

@huxuan I consider we should avoid generating a ghost author in commits history. How about just aborting the renovate job and exit or skip with a warning job message when no RENOVATE_GIT_AUTHOR setup.

I would prefer not, A warning is OK but there is no need to stop the renovate. The commit author is more like a JiePi (洁癖), it does not affect the functionality of renovate. And I want to remind considering more about the usage of template users. To ease the setup process, we should try to reduce those must-have configurations.

And I do not think it is related to this pull reuqest. This pull request just make the env RENOVATE_GIT_AUTHOR injectable. Maybe we should continue the disscussion in #400.

@msclock
Copy link
Contributor

msclock commented Mar 18, 2024

@huxuan I consider we should avoid generating a ghost author in commits history. How about just aborting the renovate job and exit or skip with a warning job message when no RENOVATE_GIT_AUTHOR setup.

I would prefer not, A warning is OK but there is no need to stop the job. The commit author is more like a JiePi (洁癖), it does not affect the functionality of renovate. And I do not think it is related to this pull reuqest. This pull request just make the env RENOVATE_GIT_AUTHOR injectable. Maybe we should continue the disscussion in #400.

All right, a warning message needs to nofify the existence of a ghost author and force maintainers to choose to modify commit author using git commit --amend --author <new author> if I don't make mistakes.

Then the pr is open to merge👌

@huxuan
Copy link
Member Author

huxuan commented Mar 18, 2024

All right, a warning message needs to nofify the existence of a ghost author and force maintainers to choose to modify commit author using git commit --amend --author <new author> if I don't make mistakes.

Yep, let us do this in a separate pull request. Feel free to create one if you are interested. :-)

@huxuan huxuan merged commit a181692 into serious-scaffold:main Mar 18, 2024
8 checks passed
@huxuan huxuan deleted the xuan.hu/inject-renovate-git-author branch March 18, 2024 03:13
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