emogrifier icon indicating copy to clipboard operation
emogrifier copied to clipboard

Update the CONTRIBUTING file

Open oliverklee opened this issue 7 years ago • 7 comments

It looks like this still leaves too many questions open.

oliverklee avatar Mar 02 '17 12:03 oliverklee

From experience, if a rebase is required, the procedure can go awry if not performed carefully. The following is a tried-and-tested procedure to rebase a ‘feature’ branch to the latest master:

  1. Make sure that your local copy of the repository has the most up-to-date revisions of master (this is important, otherwise you may end up rebasing to an older base point):

    git checkout master
    git pull
    
  2. Switch to the (feature) branch to be rebased and make sure your copy is up to date:

    git checkout feature/something-cool
    git pull
    
  3. Consider taking a copy of the folder tree at this stage, this may help when resolving conflicts in the next step.

  4. Begin the rebasing process

    git rebase master
    
  5. Resolve the conflicts in the reported files. (This will typically require reversing the order of the new entries in CHANGELOG.md.) You may use a folder ‘diff’ against the copy taken at step 3 to assist, but bear in mind that at this stage git is partway through rebasing, so some files will have been merged and include the latest changes from master, whilst others might not – in any case, you should ignore changes to files not reported as having conflicts.

    If there were no conflicts, skip this and the next step.

  6. Mark the conflicting files as resolved and continue the rebase

    git add *.*
    git rebase --continue
    

    (You can alternartively use more specific wildcards or specify individual files with a full relative path.)

    If there were no conflicts reported in the previous step, skip this step.

    If there are more conflicts to resolve, repeat the previous step then this step again.

  7. Force-push the rebased (feature) branch to the remote repository

    git push --force
    

    The --force option is important. Without it, you’ll get an error with a hint suggesting a git pull is required:

    hint: Updates were rejected because the tip of your current branch is behind
    hint: its remote counterpart. Integrate the remote changes (e.g.
    hint: 'git pull ...') before pushing again.
    hint: See the 'Note about fast-forwards' in 'git push --help' for details.
    

    DO NOT follow the hint and execute git pull. This will result in the set of all commits on the feature branch being duplicated, and the ‘patching base’ not being moved at all.

JakeQZ avatar Sep 30 '19 16:09 JakeQZ

x-ref: https://github.com/MyIntervals/emogrifier/pull/804#discussion_r332281248

JakeQZ avatar Oct 07 '19 23:10 JakeQZ

Some notes about potential pitfalls, mostly for classification/recollection at this stage as we indulge in Psalm - off the top of my head:

  • splitting PRs into manageable chunks
    • how to deal with a situation where you've made a big change on a branch and need to break it down into manageable PRs
  • always include tests if there's anything testable added
  • git CLI usage and recommended commands and methodology
  • making a good commit title and message
  • issues with GitHub UX (TBD)
    • (for collaborators) the commit title and message when squasing-and-merging

Probably lots more. These are just what spring to mind right now.

JakeQZ avatar Oct 08 '19 01:10 JakeQZ

Another one:

  • Side chat - If there's something to discuss that doesn't relate to the issue in hand, let's create a new 'issue' for it where we can discuss (one of the collaborators can mark it as 'discussion'). This would help keep a PR clean of no-longer-relevant-to-the-specific-issue conversation.

EDIT: Note that I'm not saying side-chat should be discouraged at all. I personally enjoy humorous comments, they can help make the process more enjoyable for all. Just that if a separate issue is identified, it should probably be created as a new 'issue' and the discussion can continue there.

EDIT2: And actually, thinking about it, methodology/approach is a bad example (now changed above). Discussion about that may well belong where it originated, because there's no issue with the code base to address (except for perhaps the CONTRIBUTING file - see, um, #373). I don't want to discourage any of those types of discussions (or any type of discussion, really). Just that it can sometimes be continued in a more suitable place.

JakeQZ avatar Oct 09 '19 00:10 JakeQZ

* issues with GitHub UX (TBD)

I've put in a feature request or two - see https://github.com/dear-github/dear-github/issues/362

JakeQZ avatar Oct 09 '19 00:10 JakeQZ

* Side chat

On the other side of the coin, I think friendly comments like "nice catch" or "good job" should be welcomed. Trying to strike a balance here between making PRs manageable and encouraging anyone with suggestions/improvements to contribute. (See 'edit' above, too.)

JakeQZ avatar Oct 09 '19 01:10 JakeQZ

Ths instructions for installing the development dependencies need to be updated to include installing tools with PHIVE, and with which tools are now used. Also, IIRC, we no longer need the slevomat/coding-standard package.

Info about the available Composer scripts probably needs updating too.

JakeQZ avatar May 24 '24 22:05 JakeQZ