Page MenuHomeSoftware Heritage

Correct typo "scrapping" -> "scraping"
ClosedPublic

Authored by mcv21 on Apr 23 2019, 6:01 PM.

Diff Detail

Repository
rDDOC Development documentation
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mcv21 created this revision.Apr 23 2019, 6:01 PM
mcv21 edited the summary of this revision. (Show Details)Apr 23 2019, 6:07 PM
zack accepted this revision.Apr 23 2019, 6:20 PM
This revision is now accepted and ready to land.Apr 23 2019, 6:20 PM

[I think I can't merge this myself]

zack added a comment.Apr 24 2019, 10:42 AM
In D1430#31457, @mcv21 wrote:

[I think I can't merge this myself]

You should be able to just "git push" to master, now that this revision is accepted.
Please let us know if that doesn't work.

I tried (having done git rebase -i to tidy up the commit message, and then git merge), but get refused:

mv3@deskpro108655:/upstreams/SoftwareHeritage/swh-docs$ git push
X11 forwarding request failed on channel 0
Exception: You do not have permission to push to this repository.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Oh, I'm going to have to do arc diff --update first, aren't I? Sorry, not used this tooling before, and it's a bit confusing!

mcv21 updated this revision to Diff 4653.Apr 24 2019, 10:46 AM

Tidy commit message.

Hm, no, I did that, but I still get the same permission denied message.

Hi @mcv21 , did you try to land that diff directly from the web interface by clicking on the "Land revision" link on the top left part of that page ?

@anlambert no, I don't have any such link - the top left of this page ( https://forge.softwareheritage.org/D1430 ) has "Accepted" and "Public" below the title of this Differential, but nothing else...

@anlambert That feature is only enabled on swh-storage, and I think it's broken anyway.

@mcv21 What is the output of git remote -v?

@vlorentz thus:

mv3@deskpro108655:/upstreams/SoftwareHeritage/swh-docs$ git remote -v
origin  ssh://git@forge.softwareheritage.org/source/swh-docs.git (fetch)
origin  ssh://git@forge.softwareheritage.org/source/swh-docs.git (push)

Ok, so the fact that you are not able to push is that you are not a member of the Developers phabricator project.
As you can see on the repository policies, there is a push restriction on that specific users group.

@zack, I can edit the policies but I am not comfortable with Phabricator configuration.

@anlambert non-developers have been able to push accepted diffs to master in the past

@vlorentz, looking at the commits history of the documentation repository, only team members have pushed on it (which are de facto members of the Developers project)

This revision was automatically updated to reflect the committed changes.
zack added a comment.Apr 24 2019, 2:05 PM

@mcv21: first, apologies for the conflicting communication. We don't have a lot of external contributors (yet), so our processes are not very oiled on that front (yet).

Second: good news, I've just landed your patch, no need for you to do anything else, it's done (see d146bfdc4ca246640a6e7ca6618f3acfd8a88e13). Thanks a lot for your contribution!

Third: everyone else: once a change is accepted, you can land it yourself by doing first arc diff D1430 and then arc land.
(Yes, we need to update our contribution docs accordingly.)

Third: everyone else: once a change is accepted, you can land it yourself by doing first arc diff D1430 and then arc land

I think you mean arc patch D1430 then arc land

Nevertheless, arc modify the commit message and two identical commits are generated (see https://forge.softwareheritage.org/source/swh-docs/history/master/).

zack added a comment.Apr 24 2019, 2:14 PM

Third: everyone else: once a change is accepted, you can land it yourself by doing first arc diff D1430 and then arc land

I think you mean arc patch D1430 then arc land

Yes, sorry, arc patch as documented here: https://wiki.softwareheritage.org/wiki/Code_review_in_Phabricator#Reviewing_locally_.2F_landing_someone_else.27s_changes

Nevertheless, arc modify the commit message and two identical commits are generated (see https://forge.softwareheritage.org/source/swh-docs/history/master/).

That's only because I've (erroneously, this time) used arc land instead of arc land --squash. The other manual git-based method mentioned at the above wiki page is also fine.

That's only because I've (erroneously, this time) used arc land instead of arc land --squash.

Oh I see.

The other manual git-based method mentioned at the above wiki page is also fine.

I knew about the manual landing from us but I am not a big fan of all this noise added to the commit message by arc.
Based on my understanding, if the original commit author is able to push after we accept a diff, this noise will not
be present. Anyway, that's not a blocker to integrate changes so I will also proceed like this in the future.

zack added a comment.Apr 24 2019, 2:26 PM

I knew about the manual landing from us but I am not a big fan of all this noise added to the commit message by arc.
Based on my understanding, if the original commit author is able to push after we accept a diff, this noise will not
be present. Anyway, that's not a blocker to integrate changes so I will also proceed like this in the future.

Fully agreed.

I've now filed T1689 to make sure we can arc land via the Web UI in all cases. That has the potential of easing the workflow for both external and internal contributors.
Let's continue the discussion there.