Update #3 on “Stepping back”

Edit to add this set of links to the posts in the series

In Update #2 on “Stepping back” I alleged that some of the public statements by the Tusky project, prompted by Stepping back from the Tusky project, were lies.

It is my responsibility to present evidence for that. This is one of a collection of posts that do that, or otherwise respond to the public statement by the Tusky team.

Oh, and if you're reading this and thinking “Good God, is he really going to go through the Tusky statement sentence by sentence”, no, I'm not. It's just unfortunate that the first two are sufficiently misleading that they demand an individual response.

The “TL;DR” section

In one case, this meant that there was no publicly visible work, because the contributor did not manage to get it into a usable form, despite us helping them as much as possible

This is a reference to invoice #125597 from Mike Haynes (“Z” in the project's statement, but since the invoice is public and lists Mike's name I'm using it here). Reading that invoice is what first raised my concerns.

I do not think the quoted sentence from the project statement is a lie. I think it is (a) very misleading, and (b) unfairly blames the contributor, when the project should clearly accept responsibility for the failure.

The closest the project comes to accepting responsibility for this is buried in paragraph 18 of the statement, writing “It is a failure of our project management that no one followed up on this until this month when Nik raised the issue”.

The invoice describes work on three specific GitHub issues. Reviewing the issues showed that no work had apparently been done. More accurately, no work had been done anywhere that was visible; no PRs, no comments on issues, no questions from Mike in public or private fora that I could see.

The Open Source Collective documentation provides a clear requirement for invoices:

Invoices need a clear description of services rendered and provide sufficient detail to make it clear what the collective is paying for. This is because this description may be checked by our accountant for compliance. That means that someone with no knowledge of your project, and only a limited knowledge of open source, should be able to understand what was done from the description. - Invoice and Reimbursement Examples – Open Source Collective

An invoice that lists work on three different GitHub issues, where none of those issues, or anything related to them, shows any evidence that the work was done, does not, I think, meet those requirements.

To be crystal clear: I do not think Mike intended to defraud the project, nor do I think he did defraud the project. He submitted the information he was told to submit, and the responsible people failed to follow the proper processes. I think he was failed by the project, it is terrible that he is in this position, and he deserves a public apology. I said as much in the “Tusky Contributors” channel, writing:

And to be super clear — I do not think any of this is your fault. You wrote that you had problems getting a PR submitted; when that happened I think the project should have liased with you to make that possible, possibly by escalating to any of the other contributors and asking them to assist. That's the project's failure, not yours.

During the discussion in the “Tusky Contributors” channel Mike explained:

I just want to say: this was work that was intended to be done, but unfortunately I couldn't get done, primarily due to my inability to wrap my head around getting a PR submitted. I did have code written for these issues, but I'm not sure if I still have them stashed or not.

I'm really sorry for any confusion I caused and for how I approached the OC invoice system. Conny had told me going forward if I were to submit any invoices that a PR(s) needed to be provided

I replied:

Thanks for the info. If you do still have the code, let me know, I can work with you on the PR process and getting them submitted.

Mike took me up on that offer. For approximately two hours, across two days, I worked with Mike to understand what the technical roadblocks were so that he could complete one PR and close one of the issues mentioned in the invoice. The final PR is Adding “bookmarks” tab to timeline by SomaRasu · Pull Request #3983 · tuskyapp/Tusky · GitHub, you can confirm the timeline yourself with it.

The other two issues remain open at the time I left the project.

The project's statement acknowledges this in paragraph 18, but is careful not to acknowledge that I'm the person who worked with Mike to complete the submission.

On the code side, Z wrote code toward resolving three GitHub issues; one of these was completed to the point it could become a GitHub PR, and was eventually merged this month by Nik

After working with Mike to resolve the technical issues he was having it is my opinion that the project was not “helping them as much as possible”.

First, it only took me about two hours of on-and-off discussion with Mike to resolve the problem. In my experience working with other developers on the project I'm comfortable saying any of them would also have been able to resolve the problem too.

Second, the issues were not especially difficult to resolve.

Obviously, opinions on the technical difficulty of a particular task can reasonably differ between different people. So I have placed a copy of the entirety of my conversation with Mike helping him to submit the PR below, so that you can form your own opinion about whether the project was really “helping them as much as possible” before.

The conversation is a little technical, but when viewed within the context of “Android software developers discussing a version control system” I do not think it is unreasonably so.

If you are familiar with Git and don't want to read the whole conversation the underlying issue was that Mike had cloned the project's primary repository locally, and did not have permission to push his branch to that. After repointing the origin remote in his local repository to his fork he was able to create a PR in the normal way. This was straightforward to diagnose and correct.

Wednesday, August 23rd

Mike: Hi, I'm currently trying to push my local branch to remote, but I'm currently unable due to permissions (i was sent an invitation to collaborate but the invitation expired)

Nik: That's weird. Are you trying to push to the Tusky repository, or your own fork?

Mike: For the Tusky repository. I wasn't sure which one was the preferred practice

Nik:

  1. Make your own fork of the repository
  2. Clone your fork to your computer
  3. Create a new branch (from develop) to do you work
  4. Commit the changes to your branch
  5. Push your branch to GitHub
  6. In the GitHub UI there'll be a big option to create a PR from your branch

What does git remote -v print in your local clone?

This may be faster with a video call, if you're free in about 10 minutes

Mike:

origin https://github.com/tuskyapp/Tusky.git (fetch)
origin https://github.com/tuskyapp/Tusky.git (push)
upstream https://github.com/tuskyapp/Tusky.git (fetch)
upstream https://github.com/tuskyapp/Tusky.git (data)

i cant video call at the moment, no working mic

Nik: OK. Yeah, those the wrong origin values — that means you've cloned directly from the Tusky repo.

We can probably fix this without destroying your existing work though. Give me a couple of minutes to write up some instructions.

Mike: Ahhh okay. No problem

Nik: All your work has been done on your own branch, right? Not the develop branch?

Mike: Yes, i created my own branch

Nik: In your current working copy, run ./gradlew clean. That will remove some generated files, and make the next step a lot faster.

Make sure you don't have any uncommitted changes, run git status to check. If you do, commit them to your branch (not develop).

Switch to the develop branch, make sure you've pulled the most recent changes from the Tusky repo:

git checkout develop
git pull

Make a complete copy of your current working copy directory somewhere else (e.g., if it's checked out in to a directory called Tusky, make a copy of the Tusky directory and everything in it).

The origin remote for your repo is pointing at the Tusky repo. It needs to point to your fork. To do that, run the following:

git remote set-url origin https://github.com/SomaRasu/Tusky.git

Verify that this has worked, by running git remote -v. The expected output is:

origin  https://github.com/SomaRasu/Tusky.git (fetch)
origin  https://github.com/SomaRasu/Tusky.git (push)
upstream        https://github.com/tuskyapp/Tusky.git (fetch)
upstream        https://github.com/tuskyapp/Tusky.git (push)`

Note that the origin entries have changed, the upstream entries have not.

If that worked, switch to the branch that contains your changes (git checkout ...).

Push the changes on your branch to your fork with git push.

Open https://github.com/SomaRasu/Tusky — you should see a banner saying something like “Branch X had recent pushes, do you want to create a PR?” (something like that, I forget the exact message).

(if that worked you can delete the backup of the Tusky directory)

I'm going to be away for 4 hours or so, but do let me know how you get on

Mike: Will do. I'll give it a try in a little bit. Thanks for your help!

Thursday, August 24th

Nik: How's it coming on? I'll have some more time in about 3 hours if that helps

Mike: It seems to have gone well: I was able to submit a PR to my forked copy of the repo (right now it's going through the Bitrise check). Would you like a link to the PR?

Nik: yes please

Although you shouldn't be submitting a PR to your repo. The PR should be made against the main Tusky repo.

Mike: https://github.com/tuskyapp/Tusky/pull/3983

ahh, i see. I might have misspoke

Nik: Ok, that looks like you've created the PR in the right place

On the page for your PR, there's a “Reviewers” section at the top right. You can add me as a reviewer.

You'll see a couple of things happen on the PR pretty quickly.

  1. The “CI / Build (pull_request) action will run

That's already finished. It's reported a problem with your code. You can click the “Details” link, and if you read that you'll see that the “ktlint” step has failed. Line 95 tells you exactly what the problem is.

  1. The “reviewdog-suggester” action should run.

When that completes you should see some automatic comments on your PR with suggested fixes for the code so that it will pass the lint checks. There should be a “commit suggestion” option on the comment, which will make the suggested change to your PR.

Mike: okay: I see the issue outlined in step 1: /keylesspalace/tusky/TabData.kt:122:88 Missing spacing before "}"

I just fixed that. should i commit and push that change

Nik: If you fixed it locally on your computer, yes. If you fixed it by pressing the reviewdog button then no.

Mike: okay. i didnt see the 'commit suggestion' option in reviewdog

Nik: If you open https://github.com/nikclayton/kotlin-workflow-test/pull/1, and scroll down, do you see a comment added by “github-actions”, that shows the problem and suggests the fix, with a “Commit suggestion” button?

Oh, never mind. Damn, there needs to be a config change in the Tusky repo before that will work properly

OK, yes, commit the change and push it.

In future it's a good idea to try and remember to run the ktlintCheck action to find problems, and ktlintFormat action to fix them. But don't worry too much, I forget all the time.

Mike: ahh, okay, I'll make sure to remember that

okay, just pushed. waiting on the checks to re-run

Nik: Meanwhile, up at the top right of your PR you should see a “Reviewers” section, with no reviewers listed. You can go ahead and add me.

And if this PR fixes one or more open GitHub issue you should add them as the last line of the PR description (the “...” menu at the top right of the PR description will let you edit it).

This should look like:

Fixes #1234

Or

Fixes #1234, #4567, ...

if it fixes multiple open issues.

When you do that GitHub will do two things:

  1. It adds a link from the issue back to this PR
  2. When the PR is merged it will automatically close those issues

Mike: For some reason, I dont get an option to suggest a reviewer

Nik: Ah. Today I learned that PR submitters can only add reviewers if they have write access to the repo. No worries, I'll add myself

Mike: ahh, i see

Nik: That “Fixes #2368” that you've added should be the last line on the PR description (the very first text block in the PR, not a new comment). Use the “...” menu at the top right of the description, and use the “Edit” option to change it.

Mike: Oops, sorry

Nik: No worries. And I've been able to review, approve, and merge the PR.

And if you look at https://github.com/tuskyapp/Tusky/issues/2368 you'll see that it mentions the PR, and is now marked as closed because the PR is merged.

One issue down, 454 still to go...

A handy trick is to start the names of your git branches with the ID of the issue.

So in this case something like 2368-add-bookmark-tab

That way you never have to go hunting for “Which branch did I make this particular fix on?”

Mike: Ahhh, okay. We did that at my last job. I didnt know if there was a convention for branch names

Nik: Not that the project enforces — it's purely a thing that's useful for the PR author I think, so name them however makes the most sense for you.

Mike: Gotcha

Nik: Given the discussion in “Tusky Contributors” I'd recommend a brief post in there that links to the PR too. Now that it's merged it'll be in the next Tusky nightly release, and the next full release — you don't have to do anything else to make that happen.