README/CONTRIBUTING.md

[ler762]

README/CONTRIBUTING.md could use some updating

### Using Git appropriately

 1. Fork the repository to your GitHub account.
 2. Optionally create a **topical branch**, a branch whose name is succinct but explains what you're doing, such as "feature/add-new-lines".
 3. Make your changes, committing at logical breaks.
 4. Push your work to your personal account.
 5. [Create a pull request](https://help.github.com/articles/using-pull-requests).
 6. Watch for comments or acceptance.

Is creating a branch optional or desired? I got the impression it was desired, so shouldn't it be
2. Create a **topical branch**, ... "

3. Make your changes, committing at logical breaks
I'm guessing that logical breaks means different things to different people. like maybe I'm all done editing so commit.
3. Make your changes, test, and commit when you're happy with the results.
is what I've been doing. When do you commit? or maybe better, when do you recommend committing?

Take a look at #717
Assuming there is agreement to update CONTRIBUTING.md - do I make the changes in my doc-nits branch & do another pull request or am I supposed to make another branch?

4. Push your work to your personal account.
What personal account??? My github acount from step 1?
Yeah, I know, synonyms are recommended to break the monotony in prose, but synonyms are discouraged for technical documentation where clarity is paramount.

Isn't there a step between 1 & 2 - clone the fork from your personal github account to your computer?
yeah, that's probably considered obvious. But I'm not a programmer & I don't know git, so my background knowledge is pretty much non-existent. It'd be nice if you flattened out the learning curve for newbies

[geoffmcl]

@ler762 certainly agree CONTRIBUTING.md could use some updating...

As the document more or less says, to me the first step, before a PR, is to create an issue, like you have done here, so the problem being addressed can be discussed a little, and agreed somewhat... although I suppose some simple things, like a spelling mistake does not really need that...

Yes, that Optionally should be removed... and yes, while the branch name can be long and descriptive, i.e. topical, but it does not have to be...

Your doc-nits is fine... and you will note I often use the issue-<NUM>. But the actual name is more style than substance... it is a temporary git lightweight marker that typically can/will be deleted after the merge...

Then as you no doubt realise, everything you push to that branch will be combined together in the single PR by github, so it is important to have multiple branches for multiple issues...

Like your #717 is multiple, gnu-emacs and show-filename, but it can just be tolerated under the same issue show filename on messages... so no problem...

And the jump to fixing a spelling mistake in CONTRIBUTING.md is again a stretch, but ok, it is so small... so again, no particular problem... but...

But the bigger changes needed in CONTRIBUTING.md should be separate... they could be in say a doc-nits2 branch or something...

It seems PR #717 makes PR #715 redundant, so it can be closed... can you do that, or should I?

So back to CONTRIBUTING.md enhancement...

Why not start with what I wrote in #713, here repeated and enhanced some more...

1. Fork tidy to your own github account. Use top right Fork icon.
2. Generate a SSH Key, and add it to your https://github.com/<name> settings, SSH and GPG keys
3. Clone your own fork - $ git clone git@github.com:<name>/tidy-fork.git tidy-fork
4. Create a branch - $ cd tidy-fork; $ git checkout -b <branch-name>
5. Edit, and commit your changes to this branch of your fork.
6. Test your changes, and if appropriate run regression tests.
7. Publish the branch - $ git push -u origin <branch-name. - to your remote fork.
8. Create a Pull Request, a PR, here.
9. Watch for comments, acceptance.

And then a general description on step 5., like -

Concerning editing and committing your changes, generally it is better to commit changes often, adding an appropriate commit message to each. This also aids in the PR review. But the situation varies. Like adding say an option, which can mean several files have to be edited, where it is likely appropriate to combine a considerable number of edits into one commit. There can be no hard and fast rules on this.

Please note - if you want to change multiple things that don't depend on each other, use different branches, and make sure you check the next branch back out before making more changes - that way we can take in each change separately, otherwise Github will combine all your branch commits into one PR. And see below on keeping your next fully in sync with here - this is important.

And could maybe add more words about branch naming, but may not required...

Anyway, just my initial thoughts... certainly look forward to more feedback... this is important... thanks...

[ler762]

Then as you no doubt realise, everything you push to that branch will be combined together in the single PR by github, so it is important to have multiple branches for multiple issues...

At this point, it would be a mistake to assume I realize anything about git. I DID have multiple branches after starting all over again:

* doc-nits
  doc_gnu-emacs
  next
  show-filename

and I did do three separate git push -u origin ... calls. I thought I was just pushing the typo fix to htacg/tidy-html5 but somehow all three got lumped together. sigh
Which is a long way of saying that if you think PR #715 should be closed it'd be best if you did it and I'll try to figure out how I ended up with

Branch: doc_gnu-emacs
This branch is 1 commit ahead of htacg:next.
Branch: show-filename
This branch is 2 commits ahead of htacg:next. 
Branch: doc-nits
This branch is 3 commits ahead of htacg:next. 

instead of each branch being 1 commit ahead of htacg:next

Why not start with what I wrote in #713 ...

I'm wondering about step 2
What does ssh get you? I couldn't get it to work. dunno if it was because I started off with
git clone https://github.com/ler762/tidy-html5.git
or what, but I finally gave up & left everything using https

3. Clone your own fork ...

wouldn't it be
$ git clone git@github.com:<name>/tidy-html5.git tidy-fork

... if appropriate run regression tests.

which would mean cloning https://github.com/htacg/tidy-html5-tests and then doing

$ cd tidy-html5-tests/tools-sh
$ ./testall.sh ../../tidy-html5/build/cmake/tidy
$ diff -q ../cases/testbase-expects/ ../cases/testbase-results

right?

[geoffmcl]

@ler762 I'll certainly agree git can present quite a steep learning curve, and I am still learning ;=))

I do not know exactly why you got an accumulation of commits across branches, but I guess you may have missed the words "make sure you check the next branch back out before making more changes"... and here more changes would include creating a new branch...

That is if you were still in the doc_gnu-emacs when you created the show-filename branch, then of course all commits will accumulate. Likewise, when you created the doc-nits it seems you accumulated all the previous commits... but I am guessing...

When in any particular branch a $ git log > ../temp.log will show you what commits are included in it... you can reduce what is shown by add say -5 to the command and you will only be shown the last 5, etc... and there are lots of other options to change the format, etc...

And since I have done some merges and commits since you started your forked next should show something like This branch is 6 commits behind htacg:next., like my tidy fork does at present...

And as the last paragraphs of CONTRIBUTING.md state "you will need to rebase your fork, to htacg next, before doing any more work, and likewise branches..." - my emphasis added. That is each branch in turn must be checked out and rebased to next, after a $ git fetch upstream... and conflicts cleaned up if any.

Sometimes git is great, but other time seemingly really dumb in a merge, which rebase is, and will add <<<<<< ... >>>>>> markers around things it can't merge. But usually I have found it easy to fix... usually deleting one of the 2 blocks, and possibly a minor change, and end up wondering what git exactly had the problem with...

I can reveal to you that I have two copies of my fork. One is just for experimentation, and am prepared to blow it away, clone it again, and try again and AGAIN... At that time I think git sucks, but know it as presently the best available... you need to try some others like CVS, Subversion svn, Mercurial hg, etc, to appreciate that...

I certainly think #715 has to be closed. It shows ler762 wants to merge 3 commits into htacg:next from **unknown repository** - unknown??? What does that signify? Not sure, but does not look good...

And as stated, no real problem with your #717, with all 3 commits...

What does ssh get you?

Well I assume with https you are asked each time for your password when pushing? If you create a SSH, without a passphrase, then $ git push just works... So it is mainly for convenience... nothing else... You just register the contents of the `$HOME/.ssh/*.pub file you generate...

Yes, that would be $ git clone git@github.com:<name>/tidy-html5.git tidy-fork... stupid mistake... unless in forking, or later, you re-named the repo, but agreed we should not assume that...

Being a mainly windows person not fully familiar with tools-sh, I use tools-cmd, but you seemed correct... and I think there is a run-tests.sh that does both steps for you...

As mentioned do not see a problem with the commits so far seen, but it is good to get familiar, since we do expect the regression tests to be run some time before the final merge...

HTH... thanks...

[geoffmcl]

@ler762 have just pushed some updates to CONTRIBUTING.md... commit b952e65...

Appreciated if you would check it over, and advise any errors, or other suggestions, or can this be closed... thanks...

[ler762]

@geoffmcl better, but I liked your emailed instructions more. How about this for the Using Git appropriately section

Using Git appropriately

1. Fork tidy to your own github account. Use top right Fork icon at https://github.com/htacg/tidy-html5/
2. Clone your own fork

   • using ssh
     Generate a SSH Key, and add it to your SSH and GPG keys (https://github.com/settings/keys)
     Note that if you generate the ssh key without a passphrase, operations like git push can be done without a password. Then do
     cd <some_root> # into some root directory
git clone git@github.com:<name>/tidy-html5.git

   • using https
     cd <some_root> # into some root directory
git clone https://github.com/<name>/tidy-html5.git

3. Set your user name and email address
   If you want to keep your email address private, go to https://github.com/settings/emails to find your GitHub provided no-reply email address and use that when setting your email address.
   cd tidy-html5
git config user.name <name>
git config user.email <email address>
4. Create a branch
   git checkout -b <branch-name>
5. Edit, and commit your changes to this branch of your fork.
   Please note: if you want to change multiple things that don't depend on each other, use different branches, and make sure you
   git checkout next
   before making another branch. That way we can take in each change separately, otherwise Github will combine all your branch commits into one PR.
6. Build tidy (see BUILD.md)
   cd build/cmake
cmake ../.. -DCMAKE_BUILD_TYPE=Debug
make
7. Test your changes, and if you made any code changes, run the regression tests. Very briefly, to run the regression tests
   cd <some_root> # the same root directory that was used for tidy
git clone git@github.com:htacg/tidy-html5-tests.git
cd tidy-html5-tests/tools-sh
./testall.sh ../../tidy-html5/build/cmake/tidy
diff -u ../cases/testbase-expects ../cases/testbase-results
8. Push your local changes to your github account.
   git push -u origin <branch-name>
9. Create a PR (Pull Request), here.
10. Watch for comments, acceptance.

altho should
5. Edit, and commit your changes to this branch of your fork.
be just
5. Edit the file(s) in this branch of your fork.
and then after all the testing
8. Commit your changes in this branch of your fork.
git commit -a
9. Push your local changes
--etc--

[geoffmcl]

@ler762 wow, thanks for the additional suggestions. Will certainly consider this, but first some questions...

Item 2. Clone you own fork. As you now no doubt know by now git clone git@github.com:<name>/tidy-html5.git will clone into the tidy-html5 directory. And will refuse to do anything if a tidy-html5 non empty directory already exists...

That is why I had added the tidy-fork directory at the end, sort of assuming they would have already cloned this repo, and need to use another directory name...

And when I was just a contributor to tidy, my MO was to have both. In the tidy-html5, where I never made any changes, I could do a git pull, and its results would sort of warn me that maybe I need to rebase my tidy-fork, and catch up with any upstream changes...

Of course it has the slight disadvantage that some of the scripts in say the regression tests default to ../../tidy-html5/build/cmake/tidy. But as I later learned, I should never rely on such defaults, and even now, where my tidy-html5 can have 10 or 20 testing branches, some not even pushed to the remote, each separately built in a build/temp-NNN folder, in tests I will have a t-NNN.bat which uses the correct tidy, so not really a problem...

Item 3. Set your user name and email address. Yes, maybe we should treat the user as a complete newbie, ab initio user, and no problem with this, but we should be careful how deep tidy should go into git teaching...

But you seemed to have missed the --global switch. I think what you have shown will only set it for that repo... was that your intention?

What about git config --global user.name <name>, etc? And I do not think you need to be in any particular repo for this, but no problem if you are...

Item 5. Edit, and commit - You later suggest maybe we should separate the edit from the commit, but I do not think this is necessary. Until you have done 8. git push ..., local commits can be easily undone, changed, or deleted, as you will learn. This is not so easy once pushed to the remote, i.e. published...

As indicated, in general I do think it is better to commit often, and almost never use git commit -a...

In general I prefer git commit -m "Is #NNN - what this change is about" src/<file-name> or <names>, but as also indicated this is not a hard and fast rule. And after a commit will often do git commit --amend just to read, and sometimes correct the message, or extend the message into multiple paragraphs...

But this is just my chosen preference, my style, and for sure each is free to choose their own...

And remember, before pushing, you can later squash some of the commits together, and even after the PR is created, the person doing the final merge can choose to squash all into one if there appears just too many commits, but that seldom happens...

Item 7. Test ... run the regression tests. As indicated, I mostly work in windows, but recently started running the tests in linux. I like you, had shown 2 steps, testall.sh <tidy> and diff .... No problem...

But there is also run-tests.sh which combines the 2 steps, but it sets a default tidy, which is BAD, but maybe that could be fixed to accept say run-tests.sh <tidy>... and even accept other parameter, like the tools-cmd\alltest.bat in windows, even a --help...

But that is an issue in tests... will try to remember to set that up unless someone beats me to it...

So subject to the above, as stated in general accept your expanded docs, but then probably need to change or remove some of the paragraphs following... we need to fix the whole document... any ideas on that welcome... thanks...

PS: You can change the extent to *.md.txt and upload the whole file, rather than pasting it into this issue... just an idea... I have further modified a CONTIBUTING.md.txt according to the above, attached below -

CONTRIBUTING.md.txt

What do you think... thanks...

[ler762]

Item 2. Clone you own fork. As you now no doubt know by now git clone git@github.com:/tidy-html5.git will clone into the tidy-html5 directory. And will refuse to do anything if a tidy-html5 non empty directory already exists...

That is why I had added the tidy-fork directory at the end, sort of assuming they would have already cloned this repo, and need to use another directory name...

Who's the target audience for CONTRIBUTING.md?

Go back and look at the instructions you gave me via email. As a total newbie, this is the
level of detail I was hoping to get. Remember.. I didn't even have a github account, so my background knowledge for using git & github was about zero.

On the other hand, showing how to clone something into a non-default directory and explaining why one might want to do that would be nice. But I'd add it after the numbered list and keep the basic instructions/overview at the cargo-cult level.

And when I was just a contributor to tidy, my MO was to have both. In the tidy-html5, where I never made any changes, I could do a git pull, and its results would sort of warn me that maybe I need to rebase my tidy-fork, and catch up with any upstream changes...

:) I have two separate root directories: git/tidy and git/mytidy which doesn't have

... the slight disadvantage that some of the scripts in say the regression tests default to ../../tidy-html5/build/cmake/tidy.

But as I later learned, I should never rely on such defaults

You maybe, but someone who needs to read CONTRIBUTING.md? I'd argue that as long as they're in the hand-holding stage it's best if they stay with the defaults.

Item 3. Set your user name and email address. Yes, maybe we should treat the user as a complete newbie, ab initio user, and no problem with this, but we should be careful how deep tidy should go into git teaching...

Can you do a push before setting your user name & email address? If no, this is required info

But you seemed to have missed the --global switch.

nope. If they set it locally they're not going to mess anything else up. I'm not sure you can say that if the instructions are to use the --global switch.

Item 5. Edit, and commit - You later suggest maybe we should separate the edit from the commit,
...
Until you have done 8. git push ..., local commits can be easily undone, changed, or deleted, as you will learn.

uhmmm.. the point being that I haven't learned yet, so none of this is easy. But if you think commit early; commit often is the way to go then keep the instructions the way they are :)

Item 7. Test ... run the regression tests.

It'd be nice to have a windows and *nix section for how to run the tests, but I don't know what should go in the windows section :(

we need to fix the whole document... any ideas on that welcome...

Start with who you're writing for. If you want to make it easy for newbies to contribute, I'd suggest keeping it at the cargo-cult level in the beginning of the doc.
aside- that's why I didn't put a dollar sign in front of command examples. You can't copy/paste this bit:

$ git clone git@github.com:htacg/tidy-html5-tests.git
$ cd tidy-html5-tests/tools-sh
$ ./testall.sh ../../tidy-fork/build/cmake/tidy
$ diff -u ../cases/testbase-expects ../cases/testbase-results

If you leave the dollar signs off you can.

So.. how do you want to structure the doc? Really basic copy/paste info at the beginning, with the intermediate/advanced info further down or ???

[geoffmcl]

@ler762 yes, who is the target audience is an important question... the simple answer is anybody who want to contribute to tidy development...

You, with no github account, no previous forks, are quite an unusual case. Looking back briefly at the history of PR's, it seems most contributors I looked at had come to tidy sometime after sometimes many others...

That is not to say we should not offer some help to ab initio newbie, but there should be a compromise, and not add too much information that is boring to many...

Even as I started to adopt some of your ideas to build a new file, I thought of leaving something like the original, brief, succinct list with say a sub-title like say In brief:, and then follow with your In detail: much expanded list, with multiple sub-levels, fuller explanations...

So that agrees with the idea for the document structure - start with the basics, and if the person needs more information they should read on down to the intermediate, even advanced...

By basic that means offering sound guidance, like adding the target [directory] on a forked clone, since as stated would assume they have already cloned, and built, tidy-html5... and suggest you fix/rename your 2 roots solution... but all configurations are workable...

Concerning adding the git config ..., IIRC git offers that help... insists even... and still think it should be --global... local just puts off the pain until later...

Do not quite understand your copy/paste problem with the $ shown... so again think this should stay as is... It is quite common to see, even for windows users...

And do not think we should add the windows regression test stuff... we should just point them to RUNTESTS.md... and improve that, and the tools there...

How to actually do the regression tests seems quite outside this file, and its purpose - an introduction to contributing... but for sure it should be mentioned...

So now I have talked myself out of your full expansion of the list ;=))

I am quite happy with the current improvement, and would thank you for pushing for this, and some of your suggestions...

But except perhaps for some small wording changes, even additions, I think it already goes far enough...

Uploaded is a CONTRIBUTING.md.2.txt with some further changes, best diff'ed to the current CONTRIBUTING.md, which I will shortly add -

CONTRIBUTING.md.2.txt

But as always will try to listen to further feedback... thanks...

[ler762]

test - can I use email or do I have to log into github?