Skip to content

Conversation

yardenshoham
Copy link
Member

I don't know how to test this so I'll explain my thought process:

After a discussion with @techknowlogick in cda2c38 I saw the CI config has this block:

gitea/.drone.yml

Lines 618 to 630 in cda2c38

- name: push
image: appleboy/drone-git-push
pull: always
settings:
author_email: "[email protected]"
author_name: GiteaBot
branch: main
commit: true
commit_message: "[skip ci] Updated translations via Crowdin"
remote: "[email protected]:go-gitea/gitea.git"
environment:
GIT_PUSH_SSH_KEY:
from_secret: git_push_ssh_key

I don't know much about Drone but after looking at appleboy/drone-git-push's source code, I think each setting becomes an environment variable (e.g. remote to PLUGIN_REMOTE, commit_message to PLUGIN_COMMIT_MESSAGE etc...). Take a look at the code block loading the author info:
https://github.com/appleboy/drone-git-push/blob/a69878c00665277c53fb38d6c5980221cb687935/main.go#L32-L42

Two environment variables are listed for each setting. This PR forces both to have the same value.

@yardenshoham yardenshoham changed the title Fix Drone pushing commits with bad author Fix Drone pushing commits with wrong author info Oct 14, 2022
@techknowlogick techknowlogick added type/bug topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Oct 14, 2022
Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

Thank you so much for this PR <3 Drone will fail on yaml refs across pipelines, could you instead hardcode the data instead of the ref?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 14, 2022
@yardenshoham yardenshoham force-pushed the crowdin-ci branch 2 times, most recently from 963bbaa to cba6536 Compare October 14, 2022 14:04
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 14, 2022
@techknowlogick techknowlogick merged commit 94d6d93 into go-gitea:main Oct 14, 2022
@yardenshoham yardenshoham deleted the crowdin-ci branch October 14, 2022 19:12
@yardenshoham
Copy link
Member Author

@techknowlogick can you trigger a build to test if we got it right?

@yardenshoham
Copy link
Member Author

7917123 damn 😞.

@appleboy, any idea what we're doing wrong here?

@appleboy
Copy link
Member

@yardenshoham what's the problem here?

@yardenshoham
Copy link
Member Author

We're using appleboy/drone-git-push to push commits. We are setting author_name and author_email but it's ignoring it (read PR + files changed). See commits pushed by appleboy/drone-git-push:

@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants