Skip to content
This repository has been archived by the owner on Oct 2, 2021. It is now read-only.

Document DEVELOPER instructions #289

Open
clemens-tolboom opened this issue Jun 4, 2016 · 27 comments
Open

Document DEVELOPER instructions #289

clemens-tolboom opened this issue Jun 4, 2016 · 27 comments

Comments

@clemens-tolboom
Copy link
Collaborator

clemens-tolboom commented Jun 4, 2016

Let's create a DEVELOPER.md containing all instuction for use developers.

Developer Guide

We follow http://nvie.com/posts/a-successful-git-branching-model/ apart from release branches.

So we have branches

master (protected against force pushes)
develop (protected against force pushes)
feature/* (you should push --force now and then)

Creating a feature branch

Initial

git checkout develop
git pull

Do you test work and when satified

git checkout -b feature/my-new-feature
# check your work

# commit your work

git push --set-upstream origin feature/my-new-feature

get latest develop

git checkout feature/my-new-feature
git pull --rebase origin develop

NOTE: THIS SHOULD ONLY BE DONE WITH THE APPROVAL OF THE TEAM

Creating a release

Choose the latest commit containing the release. We use annotated tags

#git checkout develop
#git checkout 7cf8330c8a77a2a6c249eded5c4d004e1c3c5721

git tag -a v1.2.8 -m "Release v.1.2.8"
@ermiyaeskandary
Copy link
Owner

@clemens-tolboom What would the CONTRIBUTING.md be used for ?

@ermiyaeskandary
Copy link
Owner

Sorry - accidental. Meant to just comment 👍

@clemens-tolboom
Copy link
Collaborator Author

clemens-tolboom commented Jun 4, 2016

What would the CONTRIBUTING.md be used for ?

For all our friends forking this repo and creating PRs like I did in the past. And of course for ourself to check and balance too.

@ermiyaeskandary
Copy link
Owner

Agreed - making...

@clemens-tolboom
Copy link
Collaborator Author

From gitter

tag -a v1.50 -m "Version 1.5 release"

Hmmm ... using SourceTree http://www.sourcetreeapp.com/ there is no message for tags

Checkout https://git-scm.com/book/en/v2/Git-Basics-Tagging#Annotated-Tags for command thanks to @ermiyaeskandary

@ermiyaeskandary
Copy link
Owner

@clemens-tolboom You can manually add the message online
The message should be a general gist of what the commits do...

@ermiyaeskandary
Copy link
Owner

@clemens-tolboom There should be a message - wierd. I go that from Git documentation 😕

@clemens-tolboom
Copy link
Collaborator Author

We will not put commit log into the release node like git log --oneline v1.2.9...v1.2.8

@clemens-tolboom
Copy link
Collaborator Author

Put release into master

git checkout master
# git merge develop

I'm not sure about the merge command. I use SourceTree

@ermiyaeskandary
Copy link
Owner

@clemens-tolboom BTW, no need to checkout to develop as it is the default branch

git checkout develop
git pull

@ermiyaeskandary
Copy link
Owner

Done ....

@clemens-tolboom
Copy link
Collaborator Author

None of the commits contains a reference to this issue :-(

And this is already on master branch WTF ... what went wrong?

@clemens-tolboom
Copy link
Collaborator Author

Please make feature/* required ... by not require it will become messy and is not according to GitFlow best practice

@clemens-tolboom
Copy link
Collaborator Author

My feature workflow (just done)

 git pull --rebase origin develop

In case of error like

Applying: Fix merge conflict on statsOverlay.style.top #275
Using index info to reconstruct a base tree...
M bot.user.js
Falling back to patching base and 3-way merge...
Auto-merging bot.user.js
CONFLICT (content): Merge conflict in bot.user.js
error: Failed to merge in the changes.
Patch failed at 0018 Fix merge conflict on statsOverlay.style.top #275

# edit the file(s) having the conflict
git diff
git commit -am "Resolve merge conflict on statsOverlay.style.top #275"

# Use the advise given by git which is either
git rebase --continue
git rebase --skip

All set? Then do a forces push

git push origin --force

@clemens-tolboom
Copy link
Collaborator Author

@ermiyaeskandary thanks for working on this :-)

@ermiyaeskandary
Copy link
Owner

ermiyaeskandary commented Jun 5, 2016

@clemens-tolboom Merged into master so we could have a working reference which can be improved - text added; no code change.

@clemens-tolboom
Copy link
Collaborator Author

But you're not following your own instructions. Release/merge to master weekly :p

@ermiyaeskandary
Copy link
Owner

@clemens-tolboom Sometimes.... Sometimes you need to override rules ;) - and by the way, it was text - not a code change so it was alright.
I guess you didn't want it to be delayed as you made the proposal ? ;) :p

@clemens-tolboom
Copy link
Collaborator Author

Can you add #289 (comment) ? Then let's close this again

@ermiyaeskandary
Copy link
Owner

So a section for merging errors with develop ?

@clemens-tolboom
Copy link
Collaborator Author

Your push is not forced which will always be the case.

And showing a failure could help.

And we missed __Each commit needs issue # __ (or I missed it while shallow reading :( )

@ermiyaeskandary
Copy link
Owner

Your push is not forced which will always be the case.

What do you mean? FORCE PUSHING IS A NONONONO

And showing a failure could help.

Seems like basic Git knowledge here but I'll add it anyway

And we missed __Each commit needs issue # __ (or I missed it while shallow reading :( )

I missed it - will add it back in..

@clemens-tolboom
Copy link
Collaborator Author

One almost always have to

git checkout featue/my-code
git pull --rebase origin develop

# work

git push --force origin featue/my-code

💯

ermiyaeskandary added a commit that referenced this issue Jun 5, 2016
@ermiyaeskandary
Copy link
Owner

ermiyaeskandary commented Jun 5, 2016

@clemens-tolboom Why? You are simply just trying to merge the lastest commits on develop into your own branch ?!

Is this what you get ?

To prevent you from losing history, non-fast-forward updates were rejected Merge the remote changes before pushing again. See the 'Note about fast-forwards' section of git push --help for details.

@ChadSki
Copy link
Collaborator

ChadSki commented Jun 6, 2016

Edit: Nevermind, force-pushing to feature branches makes sense.

@ermiyaeskandary If changes are committed to develop while you're working on feature/my-code, then there will be merges to resolve. One way of cleaning up history is "replaying" your changes on top of the new develop (using rebase). After rebasing, you have to force-push to the feature branch, because the history has been changed.

Never ever force-push to master or develop, but it does make sense for feature branches sometimes.

@ChadSki ChadSki modified the milestone: Documentation Jun 6, 2016
@clemens-tolboom
Copy link
Collaborator Author

@ChadSki indeed. thanks for the better answer.

Never ever force-push to master or develop, but it does make sense for feature branches sometimes.

That's why we have master en develop protected.

@clemens-tolboom
Copy link
Collaborator Author

Another rebase log (not sure this is useful)

clemens/Downloads/slither/Slither.io-bot % git pull --rebase origin develop
From github.com:ErmiyaEskandary/Slither.io-bot
 * branch            develop    -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: Initial task oriented example.
Applying: Remove legacy code
Applying: Allow for more food scan
Applying: Fix log priority changes.
Applying: Only log when asked for.
Applying: Changed console.log to window.log
Applying: Add task.active + documentation.
Applying: Add task CRUD ops + sort task using active too
Applying: Refactor MoveToXY + some test code added.
Applying: Move tasks into var scheduler + add init(). #275
Applying: Re-init scheduler on new game 275.
Applying: Only execute active tasks #275
Applying: Remove example code #275
Applying: Add task toggle to menu #275
Applying: Remove 'ListTasks from scheduler #275
Applying: Remove 'Add _default task #275
Applying: Remove debug line #275
Applying: Resolve merge conflict on statsOverlay.style.top #275
Using index info to reconstruct a base tree...
M   bot.user.js
Falling back to patching base and 3-way merge...
Auto-merging bot.user.js
CONFLICT (content): Merge conflict in bot.user.js
error: Failed to merge in the changes.
Patch failed at 0018 Resolve merge conflict on statsOverlay.style.top #275
The copy of the patch that failed is found in: .git/rebase-apply/patch

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".

clemens/Downloads/slither/Slither.io-bot % git diff
diff --cc bot.user.js
index bbc747b,b662fb1..0000000
--- a/bot.user.js
+++ b/bot.user.js
@@@ -1182,7 -1182,7 +1182,11 @@@ var userInterface = window.userInterfac
              var statsOverlay = document.createElement('div');
              statsOverlay.style.position = 'fixed';
              statsOverlay.style.left = '10px';
++<<<<<<< f561ebfca62be5bdae13e69c9fa2708b5b6a4e1a
 +            statsOverlay.style.top = '340px';
++=======
+             statsOverlay.style.top = '500px';
++>>>>>>> Resolve merge conflict on statsOverlay.style.top #275
              statsOverlay.style.width = '140px';
              statsOverlay.style.height = '210px';
              // statsOverlay.style.background = 'rgba(0, 0, 0, 0.5)';
clemens/Downloads/slither/Slither.io-bot % git add bot.user.js 
clemens/Downloads/slither/Slither.io-bot % git commit -m "Fixed merge conflict 'statsOverlay.style.top'"
[detached HEAD c29e3ea] Fixed merge conflict 'statsOverlay.style.top'
 1 file changed, 2 insertions(+), 2 deletions(-)
clemens/Downloads/slither/Slither.io-bot % git rebase --continue 
Applying: Resolve merge conflict on statsOverlay.style.top #275
No changes - did you forget to use 'git add'?
If there is nothing left to stage, chances are that something else
already introduced the same changes; you might want to skip this patch.

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".

clemens/Downloads/slither/Slither.io-bot % git rebase --skip
Applying: Wording #275
Applying: Removed example tasks #275
Applying: Remove _default task from menu #275
Applying: Use bot.opt.targetFps and bot.opt.foodFrames #275
Applying: Split task into Eat and CheckForFood #275
Applying: More code documentation #275 + PR #292
Applying: Added + remove console.log  #275 + PR #292
Applying: Code style @houndci-bot #275 + PR #292
Applying: Make @houndci-bot happy with return null? #275 + PR #292
Applying: Make @houndci-bot happy with return null? #275 + PR #292
Applying: More @houndci-bot fixes #275 + PR #292
Applying: Fixes parameter change #275 + PR #292
Applying: Codestyle + docs #275 + PR #292
Applying: Codestyle + docs #275 + PR #292
Applying: Remove eat task as per #292 + #275
clemens/Downloads/slither/Slither.io-bot % git push --force           
Counting objects: 99, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (99/99), done.
Writing objects: 100% (99/99), 28.27 KiB | 0 bytes/s, done.
Total 99 (delta 64), reused 0 (delta 0)
To [email protected]:ErmiyaEskandary/Slither.io-bot.git
 + 9073c3f...3f228c4 feature/task-execution -> feature/task-execution (forced update)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants