Hoppa till innehållet

Ämne på Diskussion:Developer guidelines

Saknat steg i github

11
Mattias Östmar (WMSE) (diskussionbidrag)

För att kunna lägga till en reviewr måste dessa först ha adderats som collaborators och accepterat.

Använda WMSE-gruppen kanske skulle underlätta detta?

André Costa (WMSE) (diskussionbidrag)

Även hur man resolvar conflicts i en PR (GitHub) som uppstår när en annan PR har mergats. (Svengelska galore här).

Lös dessa lokalt genom följande steg :

  1. Check out branch containing conflicts (git checkout <branch>)
  2. Pull master into branch (git pull origin master)
  3. Resolve conflicts, git add fixed files then git commit
  4. Push new version to Github (git push)

I gerrit, eller innan man pushat till GitHub kan man använda sig av git rebase.

Använder man sig av GitHub's inbyggda conflict resolver så misstänker jag att man får strul om man sedan vill lägga till fler comits i branchen. Därav ovanstående.

André Costa (WMSE) (diskussionbidrag)

Efter två diskussioner idag har jag skissat upp hela förloppet från Task till Merge på GitHub. Ta en titt och fundera på om någon del behövs läggas till i den generella beskrivningen.

Schematic flow for a coding task (on GitHub):

  1. Create a Phabricator Task for the task/bug
  2. Create and check out a branch from master (git checkout -b <branch_name>) give the branch a short but descriptive name
  3. Code in the branch (try to limit the changes to just things related to the task), commit when ready
  4. Push the commit to Github (git push)
  5. Create a Pull Request (PR)
    • Link from the PR to the Phabricator task (Task: <url> on last line)
    • Link to PR from the task
  6. Go on coding and committing/pushing in the branch until you are ready for a review
  7. Request review
    • Add reviewer to Github PR
    • Add #Patch-for-review to Task
  8. Reviewer adds review and review-cycle starts
    • Code some more
    • Review again
  9. Changes approved, ready for merge (I would suggest by the original coder)
    1. Squash-and-merge (make sure the new message contains both the description and the task link)
    2. Delete merged branch (on Github)
    3. Checkout master (locally) and pull changes (git checkout master; git pull)
    4. Delete branch locally (git branch -d <branch_name> or git branch -D <branch_name>)
    5. Add link to merge commit to Task and resolve Task (or move to done and remove #patch-for-review)
Sebastian Berlin (WMSE) (diskussionbidrag)

Man kan byta plats på punkt 5 och 6, lite beroende på hur man jobbar. D.v.s. om man gör lokala commits och bara pushar (och möjligen squashar) när det är dags för review.

Mattias Östmar (WMSE) (diskussionbidrag)

Några potentiella problem:

Steg 2. `git checkout -b <branch_name` för att skapa och checka ut till den nya branchen?

Steg 4. Om en enbart gör en "git push" i steg 4 så pushar den de facto potentiellt i en annan branch! Istället bör en enligt denna tutorial lägga till följande:

`git push origin <branch_name>`

André Costa (WMSE) (diskussionbidrag)

steg 2:

Done

steg 4:

Tror inte att detta behövs. Per default så går git push till branchen med samma namn på origin. Om en sådan branch inte finns så hojtar git till med en liten kodsnutt som du kan klipp-klistra för att att i samma veva skapa branchen. Dock måste du stå i branchen när du kör push (och <code>origin</code> måste självfallet vara länkat till github repot)
André Costa (WMSE) (diskussionbidrag)

Not1: På Mattias dator kommer detta inte upp som ett förslag. Är möjligen https vs. ssh som spökar. Undersök, om inte så lägg in hela git-anropet i beskrivningen.

Not2: Den strängen som git föreslår är git push --set-upstream origin <branch_name>

Alicia Fagerving (WMSE) (diskussionbidrag)

Note: It's possible to disable certain types of merges (Settings → Options → Merge button). If you only allow squash-and-merge, it eliminates the risk of accidentally selecting another one.

André Costa (WMSE) (diskussionbidrag)

Bör läggas till som en kommentar på någon övergripande rubrik om "setting up repo on Github" då tillsammans med kommentaren om "collaborators"

Alicia Fagerving (WMSE) (diskussionbidrag)

En not till: fetmarkera gärna att man verkligen måste stå i master när man skapar en ny branch, annars kan det bli konstigheter (man får med sig commits från den branch man befinner sig i som inte mergats till master).