Skip to content

Conversation

@jonasfranz
Copy link
Member

@jonasfranz jonasfranz commented Sep 23, 2017

This PR adds a build step for master-build. The step uploads locale_en-US.ini to crowdin via the provided API.

@tboerger This PR needs a new drone secret crowdin_key containing the API key of the gitea project. (https://crowdin.com/project/gitea/settings#api)

@tboerger Is the english source file called "locale_en-US.ini" at crowdin?

EDIT

As said on discord, maybe it makes more sense to build a drone plugin for upload and download of translations, this way anybody can prepare a malicious pull request to read the access token because I can only limit the injection to the used image and the events.

I've created a plugin for this PR and updated the .drone.yml to use this plugin. The plugin requires the crowdin key as secret CROWDIN_KEY (CC @tboerger).

Link to plugin repo: https://github.com/JonasFranzDEV/drone-crowdin
Published on Docker hub: https://hub.docker.com/r/jonasfranz/crowdin/

Working example of the plugin:

Drone: https://drone.jonasfranz.software/JonasFranzDEV/Crowdin-Demo/5
Git: https://git.jonasfranz.software/JonasFranzDEV/Crowdin-Demo
Crowdin: https://crowdin.com/project/gitea-demo/activity_stream

@lunny lunny added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Sep 23, 2017
@lunny lunny added this to the 1.3.0 milestone Sep 23, 2017
@lunny
Copy link
Member

lunny commented Sep 23, 2017

I have added secret for crowdin_key. locale_en-US.ini is the right name.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 23, 2017
@tboerger
Copy link
Member

As said on discord, maybe it makes more sense to build a drone plugin for upload and download of translations, this way anybody can prepare a malicious pull request to read the access token because I can only limit the injection to the used image and the events.

@codecov-io
Copy link

codecov-io commented Sep 23, 2017

Codecov Report

Merging #2585 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2585   +/-   ##
=======================================
  Coverage   27.33%   27.33%           
=======================================
  Files          86       86           
  Lines       17137    17137           
=======================================
  Hits         4684     4684           
  Misses      11775    11775           
  Partials      678      678

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ef8b8b...990d260. Read the comment docs.

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Copy link
Member Author

As said on discord, maybe it makes more sense to build a drone plugin for upload and download of translations, this way anybody can prepare a malicious pull request to read the access token because I can only limit the injection to the used image and the events.

I've created a plugin for this PR and updated the .drone.yml to use this plugin. The plugin requires the crowdin key as secret CROWDIN_KEY (CC @tboerger).

Link to plugin repo: https://github.com/JonasFranzDEV/drone-crowdin
Published on Docker hub: https://hub.docker.com/r/jonasfranz/crowdin/

@jonasfranz
Copy link
Member Author

jonasfranz commented Sep 23, 2017

.drone.yml Outdated

translations:
image: jonasfranz/crowdin
secrets: [ CROWDIN_KEY ]
Copy link
Member

Choose a reason for hiding this comment

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

Why secret is uppercase while others in this file are lowercase?

Signed-off-by: Jonas Franz <[email protected]>
@lafriks
Copy link
Member

lafriks commented Sep 23, 2017

LGTM you should probably submit this plugin to drone-plugins :)

@tboerger tboerger 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 Sep 23, 2017
@jonasfranz
Copy link
Member Author

@lafriks I created a PR for the plugin: drone/drone-plugin-index#71

branch: [ master ]

translations:
image: jonasfranz/crowdin
Copy link
Member

Choose a reason for hiding this comment

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

pull: true below this line

Copy link
Member Author

Choose a reason for hiding this comment

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

@tboerger Done

@daviian
Copy link
Member

daviian commented Sep 23, 2017

As I understand the plugin only uploads specified files to crowdin to make them available for translation..
But shouldn't it also download existing translations for release?

@jonasfranz
Copy link
Member Author

@daviian Yes it should, but actually it only supports uploading files. I could add downloading in future. But I think that it is enough for this PR.

@daviian
Copy link
Member

daviian commented Sep 23, 2017

LGTM

@daviian
Copy link
Member

daviian commented Sep 23, 2017

Make LG-TM work

@tboerger tboerger removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Sep 23, 2017
@tboerger tboerger added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Sep 23, 2017
@lafriks lafriks merged commit fa6d7c7 into go-gitea:master Sep 23, 2017
@jonasfranz jonasfranz deleted the translations branch September 23, 2017 16:25
@tboerger
Copy link
Member

Maybe I can start to integrate that together with Franz when this plugin gets transferred to the drone-plugins org

@lunny
Copy link
Member

lunny commented Sep 25, 2017

drone is failed, see https://drone.gitea.io/go-gitea/gitea/910/15

@jonasfranz
Copy link
Member Author

We have to add ignore_branch since we are not using branches at crowdin

@jonasfranz
Copy link
Member Author

@lunny

@lunny
Copy link
Member

lunny commented Sep 25, 2017

@JonasFranzDEV OK. So should I change some settings on crowdin or could you send another PR to fix that?

@lafriks
Copy link
Member

lafriks commented Sep 25, 2017

PR to fix that #2599

@tboerger
Copy link
Member

A note: currently this plugin doesn't commit the changes to the repository, maybe we have to add another step based on https://github.com/appleboy/drone-git-push to get these changes commited with a message including [skip ci] to avoid another CI run?

@lafriks
Copy link
Member

lafriks commented Sep 25, 2017

Downloading changes from crowdin and pushing to git would be best scenario!

@jonasfranz
Copy link
Member Author

@tboerger @lafriks I've added a section in the plugin documentation which describes the git push variant: https://github.com/JonasFranzDEV/drone-crowdin/blob/master/DOCS.md#commit-changes

@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants