Rename "Atlassian Stash" to Bitbucket

[gerhardol]

No functionality change
#3334 updated the diplay name for the plugin. Stash was still used within the code and in filenames. This was confusing both to work with "Git stashes" and Bitbucket.
Handled in #4204

How did I test this code:

• Compile - the plugin is still broken

[RussKie]

Good stuff

[RussKie]

we need to remove the existing stash.dll as well
there is a section in the definition for removal of old files

[RussKie]

Did you try generating the installer and see that it removes the old assembly?

[RussKie]

Please resolve the conflict, squash and I'll have it merged.
Thanks

[gerhardol]

It would be nice to have the merge strategies written down.
The squash should be done when merging in my preferred way of working, to see if there have been other changes that happened to be included too. Similar with what to do when other merges have been done to master that affect the PR.

[RussKie]

Yes, I agree. There are a few things missing in various areas, this is one of them.

I am of an opinion that a PR should consist of a single commit which contains a single self-contained atomic change. That can be easily reasoned about or that can be reverted, if necessary.
On occasions a PR may contain additional non-functional commits which are necessary or complimentary to the change. E.g.
commit1: update packages (non functional)
commit2: make the real change (functional)
commit3: code maintenance/refactor (non functional)

As a reviewer or a fellow developer I don't really care that it took another developer 57 commits to deliver his/her change, that it fixed typos or responded to reviews 29 times, reworked the implementation 22 times and fixed unit tests 6 times. All I care the change is done and delivered.
Personally I find it easier to review a single commit too then hunt for a change across multiple commits.

[RussKie]

Looks good

[RussKie]

Did you try generating the installer and see that it removes the old assembly?

[RussKie]

I've updated the wiki - https://github.com/gitextensions/gitextensions/wiki#committing
Feedback is welcome

[gerhardol]

I have not run the installer, failed to install it. Will try again tonight or tomorrow Den 10 dec. 2017 11:37 skrev "RussKie" <notifications@github.com>:

…

***@***.**** approved this pull request. Looks good ------------------------------ In Setup/Product.wxs <#4210 (comment)> : > @@ -723,8 +723,8 @@ <Feature Id="ReleaseNotesGenerator" Title="Release notes generator" Level="1"> <ComponentRef Id="ReleaseNotesGenerator.dll" /> </Feature> - <Feature Id="Stash" Title="Atlassian Stash integration" Level="1"> - <ComponentRef Id="Stash.dll" /> + <Feature Id="Bitbucket" Title="Atlassian Bitbucket Server integration" Level="1"> + <ComponentRef Id="Bitbucket.dll" /> Did you try generating the installer and see that it removes the old assembly? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4210 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF9Z5GrvplvS4htgv0zS-5V86CF_yoc_ks5s-7R4gaJpZM4Q8IHa> .

[gerhardol]

Tested the installer, removes Stash.dll and adds the Bitbucket.dll
So the changes in this PR should be tested (I briefly browsed the warnings for the installer, did not react on anything specific).
However, GE makes an Exception when starting.

Regarding many commits: Most of these were due to getting Mono working, I have no setup to test that, Travis did that for me... There were a directory rename added too, I would not have done that if the builds had failed already. Clean did not remove all files I had expected, I had to find all references to Stash(/|\|.dll) to find all.

@RussKie the wiki update
I would like to see the position on merging from master vs rebase (with force push) both when there are conflicting changes in master, just related or just to minimize the diff to master (to not have the maintainer doing that diff).
My opinion: Force push should never be a part of a workflow, it is evil to change the history.

Similarly, I would like to see the maintainer instructions on merging vs squashing vs rebasing the PRs. (The answer may well be "it is up to the maintainer").

Another item is how to handle "enablers" and dependent PRs. I for instance handled #4134 as a separate PR where the real change were in #4157. This was done to simplify reviews. But #4134 in itself just added dead code. I tried to explain that, but there are over 30 PRs related to #4031.
I have tried to make separate PRs but the lead time does get very long with a chain of maybe 5 chained PRs and I could maybe have split more . (I rebased maybe 30 times to prepare for #4157).

For reviewing: I would like the maintainers to encourage "voting" on issues and PRs: Reporting that a change seem reasonable should give better confidence and trying it out increases more. Users should try to increase their "standings" with testing to get their "pet" changes reviewed and maybe fixed.