Page MenuHomeSolus

Update hub to 2.14.2
ClosedPublic

Authored by yacn on May 22 2020, 8:07 PM.

Details

Summary
  • Ensure man pages are shown at full width
  • Fix subject in ci-status docs
  • Indicate in docs that you can do plain git push after hub pr checkout
  • Fix using hub inside git worktree
Test Plan
isaac@xps13 ~/packaging/hub $ which hub
which: no hub in (/home/isaac/bin:/sbin:/bin:/usr/sbin:/usr/bin:/snap/bin)
isaac@xps13 ~/packaging/hub $ sudo eopkg it hub-2.14.2-11-1-x86_64.eopkg
Password: 
Installation order: hub 
Installing hub, version 2.14.2, release 11
Extracting the files of hub
Installed hub
 [✓] Syncing filesystems                                                success
 [✓] Updating manpages database                                         success
isaac@xps13 ~/packaging/hub $ which hub
/usr/bin/hub
isaac@xps13 ~/packaging/hub $ hub --version
git version 2.26.2
hub version 2.14.2 
isaac@xps13 ~/packaging/hub $ pushd ~/git/etc/hub/ >/dev/null; hub release -L 5; popd >/dev/null
v2.14.2
v2.14.1
v2.14.0
v2.13.0
v2.12.8

Diff Detail

Repository
R1417 hub
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

yacn created this revision.May 22 2020, 8:07 PM
yacn requested review of this revision.May 22 2020, 8:07 PM
kyrios123 added a subscriber: kyrios123.EditedMay 22 2020, 9:08 PM

This summary isn't actually a summary... and it's full of things that aren't relevant to us like all the Windows specific things that should be removed.
Explanation text like hereunder shouldn't be in the summary as well

Draft pull requests are considered work in progress: they don't automatically request others for review and they are not mergeable while in their draft state.

The reference to the issues #1234 must be stripped.
I'd also remove the version headings: the users needs an overview on the major changes between the current and the new version, it is not relevant to know exactly in which version something was changed since you're bumping several versions at once.

The test plan is very light, it is just a hub --version installing the package isn't actually part of the test. The test consists of briefly demonstrating the installed package actually works.

kyrios123 requested changes to this revision.May 22 2020, 9:12 PM
This revision now requires changes to proceed.May 22 2020, 9:12 PM
yacn added a comment.May 22 2020, 9:23 PM

This summary isn't actually a summary... and it's full of things that aren't relevant to us like all the Windows specific things that should be removed.
Explanation text like hereunder shouldn't be in the summary as well

Draft pull requests are considered work in progress: they don't automatically request others for review and they are not mergeable while in their draft state.

I based my summary / commit message on the previous commits for updates to this package... which was adding the hub release notes for the version as they appear in the GitHub release as the summary. Since this bumped several major versions I figured I'd add all the release notes to be consistent.

The reference to the issues #1234 must be stripped.

why?

I'd also remove the version headings: the users needs an overview on the major changes between the current and the new version, it is not relevant to know exactly in which version something was changed since you're bumping several versions at once.

Sure, I'll fold them all into one giant list..

The test plan is very light, it is just a hub --version installing the package isn't actually part of the test. The test consists of briefly demonstrating the installed package actually works.

I followed what I read on "submitting a package" page, which reads:

If you are updating an existing package, please be sure to include a summarized changelog (or a link to the changelog provided upstream) and a test plan indicating that you’ve installed and run the package.

I did exactly that. If that's not sufficient, please update the documentation.

algent added a subscriber: algent.May 22 2020, 9:31 PM

You can add a concrete test. From the github page there is a suggestion like:

$ hub clone rtomayko/tilt
#=> git clone git://github.com/rtomayko/tilt.git

# if you prefer HTTPS to git/SSH protocols:
$ git config --global hub.protocol https
$ hub clone rtomayko/tilt
#=> git clone https://github.com/rtomayko/tilt.git

Since I assume you are using it, there should be more commands available than

$ hub --version
yacn updated this revision to Diff 21504.May 22 2020, 9:35 PM

Apply requested changes

yacn edited the summary of this revision. (Show Details)May 22 2020, 9:45 PM
yacn edited the test plan for this revision. (Show Details)
JoshStrobl requested changes to this revision.Jun 4 2020, 12:45 PM
JoshStrobl added a subscriber: JoshStrobl.
In D8928#145340, @yacn wrote:

This summary isn't actually a summary... and it's full of things that aren't relevant to us like all the Windows specific things that should be removed.
Explanation text like hereunder shouldn't be in the summary as well

Draft pull requests are considered work in progress: they don't automatically request others for review and they are not mergeable while in their draft state.

I based my summary / commit message on the previous commits for updates to this package... which was adding the hub release notes for the version as they appear in the GitHub release as the summary. Since this bumped several major versions I figured I'd add all the release notes to be consistent.

The reference to the issues #1234 must be stripped.

why?

I'd also remove the version headings: the users needs an overview on the major changes between the current and the new version, it is not relevant to know exactly in which version something was changed since you're bumping several versions at once.

Sure, I'll fold them all into one giant list..

The test plan is very light, it is just a hub --version installing the package isn't actually part of the test. The test consists of briefly demonstrating the installed package actually works.

I followed what I read on "submitting a package" page, which reads:

If you are updating an existing package, please be sure to include a summarized changelog (or a link to the changelog provided upstream) and a test plan indicating that you’ve installed and run the package.

I did exactly that. If that's not sufficient, please update the documentation.

  1. Changelogs should always be summarized, not verbatim copy / pasta. You read the changelog and use your best judgement to determine what is actually important / noteworthy.
  2. Regarding GitHub issue stripping, it's because they aren't relevant to us, aren't going to get autolinked, etc. Also...you know...because kyrios is a Global Maintainer and he told you to.
This revision now requires changes to proceed.Jun 4 2020, 12:45 PM
yacn added a comment.Fri, Jun 5, 1:17 AM

! In D8928#146289, @JoshStrobl wrote:

  1. Changelogs should always be summarized, not verbatim copy / pasta. You read the changelog and use your best judgement to determine what is actually important / noteworthy.
  2. Regarding GitHub issue stripping, it's because they aren't relevant to us, aren't going to get autolinked, etc. Also...you know...because kyrios is a Global Maintainer and he told you to.
  1. The other changelogs, that you personally approved, were copy pastes; just saying. So, again, I'm so very sorry for following the same pattern that was accepted in the seven previous releases. I will summarize the changes.
  1. Excuse me for wanting to know and understand the reasoning for doing something, which...you know...I did without waiting for any followup or answer to my question. And for the record I linked the issues myself, there was no reliance on GitHub's "autolinking" as I'm well aware that wouldn't happen on Phabricator...

You have been nothing but hostile and snarky to me since I mentioned one of the uses I had planned for the package which you personally disapproved of, plans which no longer exist by the way, while volunteering to maintain this packages; I don't appreciate it.

yacn updated this revision to Diff 21646.Fri, Jun 5, 1:40 AM

Further summarize change log

yacn edited the summary of this revision. (Show Details)Fri, Jun 5, 1:41 AM
yacn updated this revision to Diff 21647.Fri, Jun 5, 1:43 AM

pspec missing

The other changelogs, that you personally approved, were copy pastes; just saying. So, again, I'm so very sorry for following the same pattern that was accepted in the seven previous releases. I will summarize the changes.

Standards evolve and we may be more stringent in some circumstances. You'll notice with the changelogs you referenced that they are significantly more succinct and thus less problematic.

You have been nothing but hostile and snarky to me since I mentioned one of the uses I had planned for the package which you personally disapproved o

You were intending on using it for use cases we couldn't possibly condone. You proceeded to open up another task regarding the matter, with knowledge on why the package was being rejected for re-inclusion, and it was only re-opened because Bryan and you discussed the conditions in which it would be re-included and why it was being rejected. That's the only engagement I've had with you on the matter. I'm simply enforcing our standard package inclusion policy, reasonable task triaging, and reviewing of patches.

Regardless, the patch as it stands needs changes. So let's keep any further discussion focused on the patch. Thanks.

JoshStrobl accepted this revision.Fri, Jun 5, 1:49 AM

Will get this landed before sync.

yacn added a comment.Fri, Jun 5, 2:26 AM

Thank you.

You were intending on using it for use cases we couldn't possibly condone. You proceeded to open up another task regarding the matter, with knowledge on why the package was being rejected for re-inclusion.

This is not how things played out and again, I don't appreciate the way you're framing my actions. I opened the request-for-package because you closed the request-for-maintainer for a package I still needed, regardless of that one use case. But this is dragging on and dumb and there are much more important things to worry about in the world so this is the last I'll say on the matter.

In D8928#146404, @yacn wrote:

This is not how things played out and again, I don't appreciate the way you're framing my actions. I opened the request-for-package because you closed the request-for-maintainer for a package I still needed, regardless of that one use case. But this is dragging on and dumb and there are much more important things to worry about in the world so this is the last I'll say on the matter.

I'm not going to make a fuss about this, but I most certainly have chat logs from both of you, as well as the fabricator events. However, this is more or less exactly what happened:

  1. You offered to take over maintainership of hub and mentioned that you would be using it to maintain a Zoom eopkg for yourself on Github.
  2. Josh saw that you were intending to create eopkg's for Zoom, which doesn't allow redistribution under their license and rejected your offer to become maintainer on that basis.
  3. Josh went to bed.
  4. You came into IRC (rather upset if I recall) in order to engage us more directly about the decision.
  5. I explained to you that you can't legally redistribute Zoom under the terms of their license.
  6. Once you understood this, you apologized for not knowing better and removed the eopkg files from your Github repo for Zoom.
  7. After you and I came to an understanding of why Josh was upset, I told you that I would speak to Josh about reconsidering you as a maintainer.
  8. I spoke to Josh the following morning and we both agreed to accept a patch from you as the new maintainer.
  9. We informed you of this decision and you submitted the patch.
  10. Your patch is now going through the review process just like every other patch. You are a new contributor to the project and still have a few things to learn about our expectations. While I recognize that our existing documentation doesn't reflect all of these expectations, everyone on this thread has just been trying to help you to understand them.

Please recognize that Josh's initial reaction with regard to Zoom has to do with our zero-tolerance policy for license violations and other forms of copyright infringement. We carry this same policy when considering what software to reject (i.e. Popcorn Time) and which messages in our Forums and Subreddit must be removed for enabling or encouraging similar acts. It may come off as harsh, but we have a firm stance with regard to these issues as a project.

As for Josh's statement about following the feedback provided by Global Maintainers, please recognize that we all spend a fair bit of time providing feedback on community patches. It can be very frustrating when people question every little piece of feedback. Believe it or not, we do get tired of explaining things, we're only human. ;) If you would like to know more about the specifics of certain policies, I would strongly suggest stopping by IRC for a more in-depth discussion. IRC is a much better medium for having a back and forth, rather than a Patch Submission for which many other people are receiving email notifications of each change.

yacn added a comment.Fri, Jun 5, 3:31 AM

None of the above contradicts what I said about why I opened the request-for-package when the request-for-maintainer was closed. Will keep all further discussion unrelated to patches themselves to IRC. Thanks

This revision was not accepted when it landed; it landed in state Needs Review.Fri, Jun 5, 12:12 PM
Closed by commit R1417:6fbe3c84dab0: Update hub to 2.14.2 (authored by yacn, committed by JoshStrobl). · Explain Why
This revision was automatically updated to reflect the committed changes.