Differential D20759 Diff 56797 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
alvinhochunUnsubmitted
Done

s/access https/https access/

alvinhochun: s/access https/https access/
160​#. Make sure your merge request is between your branch from your fork and the official master branch.
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 ...
159#. Tell us what you've done in detail. 161​#. 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 162​
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. 163​The Krita developers will 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 contact us through the means described above.
alvinhochunUnsubmitted
Done

s/ssh/https/

alvinhochun: s/ssh/https/
164​
165​If you already have developer access, instead of working on a branch in a forked repository, you can create a new branch in the official repository and request a merge from this branch to official master.
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…
166​
167​
168​Label workflow
169​~~~~~~~~~~~~~~
170​
171​
172​.. note::
173​
174​ Work in Progress
175​
176​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:
177​
178​.. image:: ../images/en/Merge_Request_Label_Workflow.png
179​ :width: 1000px
180​
181​
182​#. 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…
183​#. When you finish your work, label it with ``Needs Review``. That will make developers know your merge request is ready.
184​#. A Krita developer will read and test your merge request. After that they will write comments and label your merge request accordingly:
185​
186​ * if the merge request is ready to be merged, with ``Approved`` label;
alvinhochunUnsubmitted
Done

IMO just write "stage all changes".

alvinhochun: IMO just write "stage all changes".
187​ * if it requires changes to proceed, with ``Needs Changes`` label.
188​
189​#. 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.
190​#. When your merge request is in the ``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.
191​
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…
192​.. note::
193​ 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 before you approve or merge!
194​
195​
196​
197​How to prepare your commits for merge request
kamathraghavendraUnsubmitted
Done

is it base or basic?

kamathraghavendra: is it base or basic?
198​~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
199​
200​After merging to master, your commits should nicely fit the Krita git history.
201​
202​* 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/>`_.
203​
204​* Every commit should be compilable and follow KDE commit guidelines - see `KDE Commit Policy <https://community.kde.org/Policies/Commit_Policy>`_.
205​
206​* Commits should be self-contained: if you code a bigger feature, it's better if you divide the code into bits that can possibly exist independently.
207​
208​* When you add new features during the development, it's fine to add new commits.
209​
210​* If you need to 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.
211​
212​.. note::
213​ You can only force-push to your own branch on your own fork, and only if the branch (or at least commits you want to rewrite) doesn't exist in the official repository yet. Don't rewrite any history in the official repo! If you need to remove changes from one of the commits that are already in the official repository, please use ``git revert``.
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.
214​
215​
216​* When you want to reduce the number of commits:
217​
218​ * you can squash them before making a merge request,
219​ * if you have developer access, you can squash the commits just before merging with master.
220​ * `Beginner's guide to rebasing and squashing <https://github.com/servo/servo/wiki/Beginner's-guide-to-rebasing-and-squashing#squashing>`_
221​
222​* Your work should go to a new branch, instead of master.
223​
224​.. fix me
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
225​ I'm not sure the next points: please fix it if it's wrong
226​
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…
227​* 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 a Krita developer.
228​
kamathraghavendraUnsubmitted
Done

follow the KDE commit guidelines

kamathraghavendra: follow the KDE commit guidelines
229​
230​
231​
232​
233​
234​
235​
236​
237​
238​
239​
240​
241​
242​
243​
244​
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 245​
163.. https://forum.kde.org/viewtopic.php?f=288&t=125955 246​.. https://forum.kde.org/viewtopic.php?f=288&t=125955
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,"
yurchorUnsubmitted
Done

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

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

Typo: procees -> proceed

yurchor: Typo: procees -> proceed
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…
yurchorUnsubmitted
Done

Typo: in Approved -> in the Approved

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

Typo: exists -> exist

yurchor: Typo: exists -> exist
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 ...
remptUnsubmitted
Done

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

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