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

Fix TMUX env variable filtering #11379

Merged
merged 1 commit into from May 14, 2021
Merged

Conversation

carlocab
Copy link
Member

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

We currently filter out TMUX, but this breaks displaying some caveats. This also enables an alias I use (improved by @Rylan12 -- thanks Rylan):

brew alias fzp='!id="$(gh pr list -L200 -R github.com/Homebrew/homebrew-core | TMUX=$HOMEBREW_TMUX fzf-tmux -p "90%,50%" --multi | cut -f1)"; [ -n "$id" ] && brew pr-publish --autosquash $id'

Alternatively, we could just stop filtering TMUX entirely. I've patched bin/brew locally to do that, but I don't know how we feel about adding more env variables to our whitelist. In particular, TMUX was mentioned in #1753 but never added, so I assume there's a reason for that.

Or, if we don't want to define HOMEBREW_TMUX, we should just remove the caveat that relies on it.

We currently filter out `TMUX`, but this breaks displaying some caveats.
This also enables an alias I use (and improved by @Rylan12):

    brew alias fzp='!id="$(gh pr list -L200 -R github.com/Homebrew/homebrew-core | TMUX=$HOMEBREW_TMUX fzf-tmux -p "90%,50%" --multi | cut -f1)"; [ -n "$id" ] && brew pr-publish --autosquash $id'
@BrewTestBot
Copy link
Member

Review period will end on 2021-05-14 at 16:14:58 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label May 13, 2021
Copy link
Member

@Rylan12 Rylan12 left a comment

Choose a reason for hiding this comment

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

Ooh, forgot that we can map TMUX directly to HOMEBREW_TMUX! Even better!

@carlocab carlocab enabled auto-merge May 13, 2021 16:42
@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label May 14, 2021
@BrewTestBot
Copy link
Member

Review period ended.

@carlocab carlocab merged commit 496c24a into Homebrew:master May 14, 2021
@carlocab carlocab deleted the tmux-env-filtering branch May 15, 2021 04:51
@github-actions github-actions bot added the outdated PR was locked due to age label Jun 15, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants