Differential D20759 Diff 56785 untranslatable_pages/intro_hacking_krita.rst

Changeset View

Standalone View

View Options

untranslatable_pages/intro_hacking_krita.rst

Show First 20 Lines • Show All 141 Lines • ▼ Show 20 Line(s)
142 142​
143So then, how does an aspiring contributor submit patches?: 143​So then, how does an aspiring contributor submit patches?:
144 144​
145Forking on Gitlab 145​Forking on Gitlab
146~~~~~~~~~~~~~~~~~ 146​~~~~~~~~~~~~~~~~~
147 147​
148.. note:: 148​.. note::
149 149​
150 Work In Progress. 150​ Work In Progress.
kamathraghavendraUnsubmitted
Done

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

Also till when do we need this flag?

kamathraghavendra: Can this be - "This page is still work in progress and might get updated later" Also till when…
tymondAuthorUnsubmitted
Done

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

tymond: I removed it... not sure if the current version is the last one, but it looks pretty final now.
151 151​
152#. Forking on gitlab is done by going to the `repository`_ and pressing :guilabel:`fork`. You will then make a personal fork of the repository. 152​#. Forking on gitlab is done by going to the `repository`_ and pressing :guilabel:`fork`. You will then make a personal fork of the repository.
153#. In your fork, you press :guilabel:`clone` to get the git urls to do the ``git clone`` from. You can then pull and push your commits from these. 153​#. In your fork, you press :guilabel:`clone` to get the git urls to do the ``git clone`` from. You can then pull and push your commits from these.
154 154​
155 You can also use the :guilabel:`Web IDE` to make your changes on invent.kde.org, but because Krita is a cpp program, we don't recommend this outside of typo fixes and doxygen. You wouldn't be able to see the effect of your changes, after all! 155​ You can also use the :guilabel:`Web IDE` to make your changes on invent.kde.org, but because Krita is a cpp program, we don't recommend this outside of typo fixes and doxygen. You wouldn't be able to see the effect of your changes, after all!
156 156​
157#. Make your first fix, push everything to your fork. 157​#. Create a new branch.
158​#. Make your first fix, push everything to your branch in your fork.
kamathraghavendraUnsubmitted
Done

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,

kamathraghavendra: Set up new remote which points to the official repository and then you will be able to update…
tymondAuthorUnsubmitted
Done

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: I changed it to: #. Set up a new remote which points to the official repository, so you'll be…
158#. Once you're done, go to :menuselection:`merge requests` and press :guilabel:`new merge request` 159​#. Once you're done, go to :menuselection:`merge requests` and press :guilabel:`new merge request`
kamathraghavendraUnsubmitted
Done

for access through https

kamathraghavendra: for access through https
kamathraghavendraUnsubmitted
Done

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

kamathraghavendra: Once you are done, login to the KDE gitlab instance and got to ...
alvinhochunUnsubmitted
Done

s/access https/https access/

alvinhochun: s/access https/https access/
159#. Tell us what you've done in detail. 160​#. Tell us what you've done in detail.
kamathraghavendraUnsubmitted
Done

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.

kamathraghavendra: Write a detailed description about the changes that you are proposing with your merge request.
160 161​
161The Krita developers be informed of new merge requests, and will try to review your request as soon as possible. If you suspect your patch slipped through the cracks, don't hesitate to make contact us through the means described above. 162​The Krita developers be informed of new merge requests, and will try to review your request as soon as possible. If you suspect your patch slipped through the cracks, don't hesitate to make contact us through the means described above.
remptUnsubmitted
Done

Add 'will' before 'be informed'

s/make contact/contact/

rempt: Add 'will' before 'be informed' s/make contact/contact/
kamathraghavendraUnsubmitted
Done

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.."

kamathraghavendra: Does notified sound good instead of informed, since it will trigger a notification via mail?
162 163​
alvinhochunUnsubmitted
Done

s/ssh/https/

alvinhochun: s/ssh/https/
164​If you already have a developer access, instead of working on a branch in a forked repository, you can create a new branch in official repository and request a merge from this branch to official master.
yurchorUnsubmitted
Done

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

yurchor: Typo: a developer -> developer Typo: in official -> in the official
alvinhochunUnsubmitted
Done

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

alvinhochun: I suggest to strongly recommend using a personal forked repo over pushing to the official repo…
165​
166​
167​Label workflow
168​~~~~~~~~~~~~~~
169​
170​
171​.. note::
172​
173​ Work in Progress
174​
175​Make sure the state of your merge request is labeled correctly. The picture below shows the base label workflow that your merge request should go through:
176​
177​.. image:: ../images/en/Merge_Request_Label_Workflow.png
178​ :width: 1000px
179​
180​
181​#. When you create a merge request, mark it with WIP to make sure no one will accidentally merge your request prematurely.
alvinhochunUnsubmitted
Done

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?

alvinhochun: I want to suggest adding `--ff-only` but assuming the reader won't be committing to their…
182​#. When you finish your work, label it with `Needs Review`. That will make developers know your merge request is ready.
183​#. A Krita developer will read and test your merge request. After that they will write comments and label your merge request accordingly:
184​
185​ * if the merge request is ready to be merged, with `Approved` label;
186​ * if it requires changes to procees, with `Needs Changes` label.
yurchorUnsubmitted
Done

Typo: procees -> proceed

yurchor: Typo: procees -> proceed
alvinhochunUnsubmitted
Done

IMO just write "stage all changes".

alvinhochun: IMO just write "stage all changes".
187​
188​#. If your merge request is in `Needs Changes` state, please follow the instruction of the reviewer and submit the code to your merge request. Add `Needs Review` label to your MR again.
189​#. When your merge request is in `Approved` state, you can either merge the code yourself to master (if you have developer access) or wait for KDE developer to do it for you.
yurchorUnsubmitted
Done

Typo: in Approved -> in the Approved

yurchor: Typo: in `Approved` -> in the `Approved`
kamathraghavendraUnsubmitted
Done

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

kamathraghavendra: This can be - code yourself to master, if you have developer access, or ...
190​
kamathraghavendraUnsubmitted
Done

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?

kamathraghavendra: We can also suggest the delete current fork and refork strategy, if gitlab gives a button in…
191​.. note::
192​ If you have developer access and merge someone's merge request to the repository, you are partially responsible for the code. Don't merge MRs that weren't approved! Read and test extensively all MRs you approve or merge!
kamathraghavendraUnsubmitted
Done

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

kamathraghavendra: Can - "Read and test extensively all MRs you approve or merge!" be "Read and test extensively…
193​
194​
195​
196​How to prepare your commits for merge request
kamathraghavendraUnsubmitted
Done

is it base or basic?

kamathraghavendra: is it base or basic?
197​~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
198​
199​.. edit me
200​ ...
201​
202​After merging to master, your commits should nicely fit the Krita git history.
203​
204​* Commit messages should state clearly what that commit changes and why - see `How to Write a Git Commit Message <https://chris.beams.io/posts/git-commit/>`_.
205​
206​* Every commit should be compilable and follow KDE commit guidelines - see `KDE Commit Policy <https://community.kde.org/Policies/Commit_Policy>`_.
207​
208​* Commits should be self-contained: if you code a bigger feature, it's better if you divide the code into bits that can possibly exists independently.
yurchorUnsubmitted
Done

Typo: exists -> exist

yurchor: Typo: exists -> exist
209​
210​* When you add new features during the development, it's fine to add new commits.
211​
212​* If you only fix previous commits, don't add new ones - instead, amend the ones that you made before and force-push your new commits to the branch in your fork.
kamathraghavendraUnsubmitted
Done

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

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

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.

alvinhochun: IMO it should be acceptable to force-push to a branch on the main repo if: 1. it's a personal…
tymondAuthorUnsubmitted
Done

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

tymond: All branches are protected, so there is no option to force-push to main repo.
213​
214​* When you want to reduce the number of commits:
215​
216​ * you can squash them before making a merge request,
217​ * if you have developer access, you can squash the commits just before merging with master.
218​ * `Beginner's guide to rebasing and squashing <https://github.com/servo/servo/wiki/Beginner's-guide-to-rebasing-and-squashing#squashing>`_
219​
220​* Your work should go to a new branch, instead of master.
221​
222​.. fix me
223​ I'm not sure the next points: please fix it if it's wrong
kamathraghavendraUnsubmitted
Done

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

kamathraghavendra: fit the Krita git history -> fit in the Krita's git histoy
224​
225​* Your commits will be rebased and put on master using fast-forward merge. If you need a manual merge (if, for example, you're working on a big feature) and you don't have the commit access, please contact Krita developer.
remptUnsubmitted
Done

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

rempt: s/contact krita developers/contact a krita developer/g
kamathraghavendraUnsubmitted
Done

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

kamathraghavendra: there are two spaces between should and state. can this be - Commit messages should clearly…
226​
227​
kamathraghavendraUnsubmitted
Done

follow the KDE commit guidelines

kamathraghavendra: follow the KDE commit guidelines
228​
229​
230​
231​
232​
233​
234​
235​
236​
237​
238​
239​
240​
241​
242​
243​
163.. https://forum.kde.org/viewtopic.php?f=288&t=125955 244​.. https://forum.kde.org/viewtopic.php?f=288&t=125955