Add gitlab workflow to Untranslatable Pages
ClosedPublic

Authored by tymond on Apr 22 2019, 11:29 PM.

Details

Summary

This patch adds the description of how one should
use labels to control the flow of merge requests.
This patch also contains information how should
one prepare commits before merging with master.

Diff Detail

Repository
R1012 Krita.org Documentation Website
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tymond created this revision.Apr 22 2019, 11:29 PM
Restricted Application added a project: Krita: Manual. · View Herald TranscriptApr 22 2019, 11:29 PM
Restricted Application added a reviewer: Krita: Manual. · View Herald Transcript
tymond requested review of this revision.Apr 22 2019, 11:29 PM

Hi @tymond Can I request you to run the image through pngquant and optipng, it would reduce it to 23 KB.

Many thanks in advance for fixing these minor typos.

untranslatable_pages/intro_hacking_krita.rst
330

Typo: a developer -> developer
Typo: in official -> in the official

352

Typo: procees -> proceed

355

Typo: in Approved -> in the Approved

374

Typo: exists -> exist

untranslatable_pages/intro_hacking_krita.rst
355

This can be - code yourself to master, if you have developer access, or ...

358

Can - "Read and test extensively all MRs you approve or merge!" be "Read and test extensively all MRs before you approve or merge!"

378

Will this sound good if it is "If you need to only fix previous commits,"

rempt added a subscriber: rempt.Tue, Apr 23, 6:37 AM
rempt added inline comments.
untranslatable_pages/intro_hacking_krita.rst
328–329

Add 'will' before 'be informed'

s/make contact/contact/

391

s/contact krita developers/contact a krita developer/g

tymond updated this revision to Diff 56791.Tue, Apr 23, 6:50 AM

Fixed typos (thanks for pointing them out!).
Optimized png image.

tymond updated this revision to Diff 56792.Tue, Apr 23, 6:53 AM
tymond marked 8 inline comments as done.

Fixed one more typo.

tymond marked an inline comment as done.Tue, Apr 23, 6:56 AM
tymond updated this revision to Diff 56797.Tue, Apr 23, 7:45 AM

Added more information about force-push.
Changed the look of labels.
Added note to watch out what MR is one trying to make.

Hi, @tymond!

The text looks very nice, detailed and precise! :) I don't see any problems in it. If we find any problems/inconsistencies, we can always adjust it on the fly :)

rempt accepted this revision.Tue, Apr 23, 10:13 AM
This revision is now accepted and ready to land.Tue, Apr 23, 10:13 AM
alvinhochun added inline comments.
untranslatable_pages/intro_hacking_krita.rst
378

IMO it should be acceptable to force-push to a branch on the main repo if:

  1. it's a personal branch (i.e. branch name starts with person_name/); and
  2. only one person (i.e. you) are working on it.

Otherwise, if we are to disallow force-pushing on the main repo even for personal branches, we should change the text in the above section to just ask everyone to create merge requests from their personal fork and not push any WIP on the main repo. We should also make sure it is disallowed in GitLab by making all branches "protected" by default.

@kamathraghavendra and @rempt: Be sure to push this :)

It can go into the master branch, as the untranslatable pages shouldn't be a bother to translators.

To push it, either use arcanist:

arc patch D20759
arc land

or use git --author

git apply <path/to/>D20759.diff
make html #just in case for errors
git add .
git commit --author="Agata Cacko <tamtamy.tymona@gmail.com>"

Then write a commit message or copy paste the summary here.
And ensure the commit message ends with
Differential Revision: https://phabricator.kde.org/D20759

There are some other things that I'm still working on, so please don't push until I say that's all....

Okay, if you're still working on it, be sure to include your copyright into the header, I think :)

tymond added inline comments.Tue, Apr 23, 12:27 PM
untranslatable_pages/intro_hacking_krita.rst
378

All branches are protected, so there is no option to force-push to main repo.

tymond updated this revision to Diff 56816.Tue, Apr 23, 12:43 PM

Added git commands.

tymond marked 2 inline comments as done.Tue, Apr 23, 12:45 PM
alvinhochun added inline comments.Tue, Apr 23, 1:08 PM
untranslatable_pages/intro_hacking_krita.rst
163

s/access https/https access/

173

s/ssh/https/

196

IMO just write "stage all changes".

330

I suggest to strongly recommend using a personal forked repo over pushing to the official repo for creating merge requests.

347

I want to suggest adding --ff-only but assuming the reader won't be committing to their master branch then it shouldn't be needed?

some suggestios

untranslatable_pages/intro_hacking_krita.rst
151–155

Can this be - "This page is still work in progress and might get updated later"

Also till when do we need this flag?

163

for access through https

167–168

Set up new remote which points to the official repository and then you will be able to update your master branch.

this is just a suggestion,

167–168

Once you are done, login to the KDE gitlab instance and got to ...

168–327

Write a detailed description about the changes that you are proposing with your merge request. If it is a change in the user interface it would be good if you can provide screenshots through attachments.

328–329

Does notified sound good instead of informed, since it will trigger a notification via mail?

"and will try to review your" can be "and they will try to review your.."

356

We can also suggest the delete current fork and refork strategy, if gitlab gives a button in the merge request page to delete the fork once the merge request is accepted, like github does?

362

is it base or basic?

389

fit the Krita git history -> fit in the Krita's git histoy

391

there are two spaces between should and state.

can this be -
Commit messages should clearly and concisely state what changes you made with that particular commit and why

393

follow the KDE commit guidelines

tymond marked 16 inline comments as done.Tue, Apr 23, 2:23 PM
tymond added inline comments.
untranslatable_pages/intro_hacking_krita.rst
151–155

I removed it... not sure if the current version is the last one, but it looks pretty final now.

167–168

I changed it to:

#. Set up a new remote which points to the official repository, so you'll be able to update your master branch.

tymond marked 4 inline comments as done.Tue, Apr 23, 2:24 PM
tymond updated this revision to Diff 56827.Tue, Apr 23, 2:40 PM

Fixed issues mentioned in comments above.

This revision was automatically updated to reflect the committed changes.