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.

Sign up for GitHub

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

Jump to bottom

pod2man: further tweak logic. #11212

Merged
merged 1 commit into from
Apr 21, 2021
Merged

pod2man: further tweak logic. #11212

merged 1 commit into from
Apr 21, 2021

Conversation

MikeMcQuaid
Copy link
Member

  • prioritise the first pod2man in the PATH if possible. This shim was created to handle the case where there isn't one but, if there is, we want to allow e.g. using a pod2man dependency to override the use of the system version
  • make /usr/bin/pod2man lower priority but still prioritise it over a Homebrew-installed pod2man that's not in the PATH unless it doesn't exist.

@MikeMcQuaid
pod2man: further tweak logic.
df9ba95 
- prioritise the first `pod2man` in the `PATH` if possible. This shim
  was created to handle the case where there isn't one but, if there is,
  we want to allow e.g. using a `pod2man` dependency to override the use
  of the system version
- make `/usr/bin/pod2man` lower priority but still prioritise it over
  a Homebrew-installed `pod2man` that's not in the `PATH` unless it
  doesn't exist.
@MikeMcQuaid MikeMcQuaid added the critical Critical change which should be shipped as soon as possible. label Apr 21, 2021
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

1 similar comment
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

@MikeMcQuaid MikeMcQuaid mentioned this pull request Apr 21, 2021
Closed
@Bo98
Copy link
Member

Bo98 commented Apr 21, 2021 •
edited

  • prioritise the first pod2man in the PATH if possible.

Honestly, this means we're one step away from removing this entirely. I'm not sure if I see any benefit of keeping it?

@MikeMcQuaid
Copy link
Member Author

Honestly, this means we're one step away from removing this entirely. I'm not sure if I see any benefit of keeping it?

I'm not sure I agree. That was the original usage/intent: handle the case where there's no versioned pod2man in the path. If you can confirm that Yosemite provides an unversioned /usr/bin/pod2man: yeh, let's 🔥 it.

@Bo98
Copy link
Member

Bo98 commented Apr 21, 2021

handle the case where there's no versioned pod2man in the path

Fair enough, though from some web searching apparently some had it but it wasn't executable.

If you can confirm that Yosemite provides an unversioned /usr/bin/pod2man: yeh, let's 🔥 it.

I'll download the Yosemite Vagrant box now. This question has come up before and I think it's going to be useful to have it on standby to test things.

@MikeMcQuaid
Copy link
Member Author

I'll download the Yosemite Vagrant box now. This question has come up before and I think it's going to be useful to have it on standby to test things.

Thanks ❤️. Will merge this as-is for now but I'm good to 🔥 if we no longer need it.

@MikeMcQuaid MikeMcQuaid merged commit e59f93c into Homebrew:master Apr 21, 2021
@MikeMcQuaid MikeMcQuaid deleted the pod2man_tweaks branch April 21, 2021 12:03
@Bo98
Copy link
Member

Bo98 commented Apr 21, 2021

Yosemite:

$ ls -l /usr/bin/pod2man*
-rwxr-xr-x  38 root  wheel    811 Sep  9  2014 /usr/bin/pod2man
-rwxr-xr-x   1 root  wheel  11611 Sep  9  2014 /usr/bin/pod2man5.16
-rwxr-xr-x   1 root  wheel  13700 Sep  9  2014 /usr/bin/pod2man5.18

@MikeMcQuaid
Copy link
Member Author

@Bo98 Cool. 👍🏻 to 🔥

@github-actions github-actions bot added the outdated PR was locked due to age label May 22, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BrewTestBot BrewTestBot BrewTestBot approved these changes

@Bo98 Bo98 Awaiting requested review from Bo98

@carlocab carlocab Awaiting requested review from carlocab

Assignees
No one assigned
Labels
critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@MikeMcQuaid @BrewTestBot @Bo98