Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added scripts for outdated appcasts #18237

Merged
merged 1 commit into from
Feb 16, 2016
Merged

added scripts for outdated appcasts #18237

merged 1 commit into from
Feb 16, 2016

Conversation

vitorgalvao
Copy link
Member

To run these, simply do it. It doesn’t matter where, they take care of everything. How they work:

find_outdated_appcasts:

  • Loop through every cask.
    • If it has an appcast, verify it.
      • If outdated, open an issue with details and outdated appcast label.

Snippet of output:

→ ~/Desktop/find_outdated_appcasts
Cloning homebrew-cask…
Verifying appcast checkpoint for 33-rpm…
Verifying appcast checkpoint for a-better-finder-attributes…
Verifying appcast checkpoint for a-better-finder-rename…
Verifying appcast checkpoint for accessmenubarapps…
Verifying appcast checkpoint for acorn…
Verifying appcast checkpoint for activity-audit…
Verifying appcast checkpoint for actotracker…
Verifying appcast checkpoint for adapter…
Verifying appcast checkpoint for adguard…
Verifying appcast checkpoint for adium…
adium is outdated. Opening issue in homebrew-cask…
Opened issue: https://github.com/caskroom/homebrew-cask/issues/17851.
Verifying appcast checkpoint for adventure…
Verifying appcast checkpoint for aerial…
Verifying appcast checkpoint for aether…
Verifying appcast checkpoint for air-connect…
Verifying appcast checkpoint for air-video-server-hd…

… Many lines later …

Some casks have appcasts that errored out, and may need to be rechecked:
homebrew-cask/butler
homebrew-cask/moom
homebrew-cask/name-mangler
homebrew-cask/witch
homebrew-cask/wordpresscom

fix_outdated_appcasts:

  • Grab all issues (no PRs) with label outdated appcast and loop through them.
    • Open their appcast in the browser (or whatever default app).
    • Initiate cask-repair (it will blind-submit).
      • If successful (i.e. if it submits the PR), close the issue that originated it.

merge_outdated_appcasts:

  • Grab all PRs (no issues) with label outdated appcast and loop through them.
    • Check if CI passed.
      • If it did, fastmerge it.
  • Again, grab all PRs (no issues) with label outdated appcast.

Snippet of output:

Updating homebrew-cask…
Merging pull request for kube-cluster…
Applying #17915 as patch…
Merged and closed issue.
Merging pull request for macspice…
Applying #17916 as patch…
Merged and closed issue.
Merging pull request for peakhour…
Applying #17917 as patch…
Merged and closed issue.
Merging pull request for pngcommentator…
Applying #17918 as patch…
Merged and closed issue.
To https://github.com/caskroom/homebrew-cask.git
 - [deleted]         update-fauxpas
 - [deleted]         update-kube-cluster
 - [deleted]         update-macspice
 - [deleted]         update-peakhour
 - [deleted]         update-pngcommentator

You might notice find_outdated_appcasts outputs full URLs (as does fix_outdated_appcasts, through cask-repair), while merge_outdated_appcasts (through fastmerge) does not. However, that will indeed happen if there’s an error. So opening issues and errors (i.e. when you might want to directly go to the issue) produces the full link, while closing them successfully does not. Showing a bunch of links in a row (i.e. a lot of merging successes) is confusing and makes it harder for the eyes to parse, but when spaced out (occasional opening of issues and PR submissions) calls attention to them. Quick tip to iTerm2 users, +click on a URL will open it.

As for the duplication between the scripts, this is one of those cases where they all do similar things but require minute differences in various areas, and jumbling them all together, at least in this initial step, would cause more confusion than what is worth.

There may also be some doubts regarding interacting directly with upstream instead of using forks, and again this is one of those cases of “first lets see if we can get it to work at all”. Doing it this way, at least initially, is easier for various reasons, including not having to worry if the maintainer has all the repos tapped/cloned and not having to worry about the naming they gave to their branches.

The final consideration is when and how to run these. I’ve been doing it so successfully for a while, but from the moment any maintainer can do it, we risk stepping on toes. I’ve been thinking of systems where the script would assign the issue to who ran it, and only run if issues were assigned to you, hence mitigating possible clashes, but all solutions require a lot of web requests, slow down operations considerably, and are convoluted. Easiest way I can think is: whoever ran the find script commits to running the other two that same day. That way, one person at a time is responsible.

Now, who to run them and when? Initially I thought about having a bot run both the find and merge scripts regularly (once or twice a week), but there are some considerations. Although I do have a server I could employ for this, it’s shared, and I’m not sure we should trust it with such responsibility1. Also, having the bot open issues when there’s no one to run the fix script soon after leaves us with a mess of issues.

So if we decide a human is to run all three scripts, we need a way to let others know when we do so, to avoid stumbling on each other.

Until we come up with a system to do so (suggestions very welcome), I don’t mind being the one to do it. I’m guessing some of you might be curious to try them at least once before, though, so if you are, reply to this issue (so others know not to run the script as well) and go for it.


1 Yes, it is git and we can reverse things, but I’m more worried about malicious commits we fail to catch. Unfortunately, there’s no way in Github to have a maintainer that can only handle issues (we would need the bot to be a maintainer to label issues). We could forego the labeling mechanic entirely and have the bot with no permissions just open issues with a certain name we can grab, but that is prone to errors.


http_status="$(curl "${curl_flags[@]}" --head --write-out '%{http_code}' "${appcast_url}" -o '/dev/null')"

[[ "${http_status}" == 200 ]] && echo 'true' || echo 'false'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could leave off the echos here and just return [[ "${http_status}" == 200 ]].

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

@vitorgalvao
Copy link
Member Author

As a small note, you may notice the find script no longer posts the full appcast to the issue. It simplifies the script, requires less requests, is faster, and since the fix script exists, it didn’t serve much purpose.

@adidalal
Copy link
Contributor

adidalal commented Feb 3, 2016

@vitorgalvao We could always use the gitter chatroom (maintainers have their own room), but I'm not sure if everyone uses it.

@vitorgalvao
Copy link
Member Author

@adidalal I’m fine with that.

@adidalal adidalal mentioned this pull request Feb 6, 2016
8 tasks
@tsparber
Copy link
Contributor

tsparber commented Feb 6, 2016

Just a note:

Initially I thought about having a bot run both the find and merge scripts regularly (once or twice a week)

For casks with unversioned urls this might no be often enough (as they are most likely not only outdated, but broken when the appcast changes).

Btw nice scripts 👍

Regarding a bot server: I've successfully used brew cask _stanza on a linux server with linux brew (which might be cheaper than a osx one). There is some env config needed to get the paths stright, but then it works like a charm 😃

@adidalal
Copy link
Contributor

adidalal commented Feb 9, 2016

Ran through it once today (for homebrew-cask only). Thoughts:

Definitely easier than manually going through things, no doubt! 👍 for scripting.
The issue opened should have a link to the homepage - I find it useful on many an occasion, for manual checking
Wording on "closed in favor of" should be improved to be clearer.
A small 'cask-repair` enhancement, made an issue in your tiny-scripts repo.
Tag does not get consistently applied, but possibly network issues are to blame there.

A question: How do you get .atom/feeds to open up in sublime text/atom/$EDITOR - I had to manually wget/open in sublime, which was not fun.

A tip: after a huge bunch of merges, do git gc; git repack -Ad; git prune to compact .git

@vitorgalvao
Copy link
Member Author

Ran through it once today (for homebrew-cask only).

So you interrupted it? homebrew-cask is the longest one, the others are relatively quick.

The issue opened should have a link to the homepage - I find it useful on many an occasion, for manual checking

But are you going through the issues, though? Why? With the fix script you don’t even need to go through the issues (you just need them to exist). Isn’t it more work to do it that way (since you’ll have to search for the correct one)? Personally, if I need the homepage, I

Quick tip to iTerm2 users, +click on a URL will open it.

I can certainly add the homepage to the issue if you want. I’m just trying to figure out a solution that would fit you better. If not and you still want the homepage in the opened issue, say so and I’ll add it.

Wording on "closed in favor of" should be improved to be clearer.

What do you suggest? I haven’t given much though to it and don’t really care. The message is just there as a way to justify why the issue was closed, in case a user finds it. I’m certainly open to changing it.

How do you get .atom/feeds to open up in sublime text/atom/$EDITOR

They open in Chrome by default, for me
image

Seems to not work in Safari, though, now that I tried it. Reading that atom file is harder than looking at the releases page, anyway, so I was already thinking of simply opening the releases page instead of the appcast when it detects it’s a github released URL, so I’ll likely just do that and solve your issue in the process.

A tip: after a huge bunch of merges, do git gc; git repack -Ad; git prune to compact .git

Will do.

@vitorgalvao
Copy link
Member Author

A tip: after a huge bunch of merges, do git gc; git repack -Ad; git prune to compact .git

Actually, can I get a bit of reading material on why those commands are the best (I was reading on them all to find out how they exactly work)?

Apart from this StackOverflow answer (with a relatively low amount of up votes), I don’t see that sequence anywhere else, and it seems that git gc already runs git prune, so it shouldn’t be needed.

From what I can find, it seems git gc should be sufficient.

@adidalal
Copy link
Contributor

adidalal commented Feb 9, 2016

I open the issues to get the latest version. I use Safari, so the app cast doesn't open up automatically. I then need to wget the app cast and open in sublime. For my ease of use I commented in the issue, so that I have that when cask repair asks.

Putting the url/github releases would be super useful, at least for me. If you can envision a better workflow, though, let me know.

"Resolved in PR #____" could work, but doesn't really matter, as you said.

I had that sequence in my .gitconfig and can't source it, so I'm assuming just gc is fine. (Will also update my dotfiles accordingly)

@vitorgalvao
Copy link
Member Author

I was already thinking of simply opening the releases page instead of the appcast when it detects it’s a github released URL

Done.

Also, the merge script now runs git gc.

vitorgalvao added a commit that referenced this pull request Feb 16, 2016
added scripts for outdated appcasts
@vitorgalvao vitorgalvao merged commit 1a41fca into Homebrew:master Feb 16, 2016
@vitorgalvao vitorgalvao deleted the outdated-appcasts branch February 16, 2016 15:39
@adidalal adidalal removed the awaiting maintainer feedback Issue needs response from a maintainer. label Apr 12, 2016
@miccal miccal removed the meta label Dec 23, 2016
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants