No shallow taps

[cnnrmnn]

• 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?

---

See #11328.
Removes the --full and --shallow tap command flags. Changes tap command to fully clone taps and convert any existing shallow clones to full clones when updated.

[MikeMcQuaid]

Makes sense to me, nice work @cnnrmnn! Won't merge just yet in case other maintainers have thoughts.

[Bo98]

Does this need a deprecation path?

[Rylan12]

Does this need a deprecation path?

Yes I think it does. I wouldn't be surprised if there are scripts tat use brew tap --shallow.

[MikeMcQuaid]

Nice work, almost there!

[cnnrmnn]

Nice work, almost there!

Appreciate you taking the time to help me out. All changes noted for future contributions :)

[Rylan12]

Two comments:

First, can you just add a few TODO comments wherever the commented out deprecations are? It makes it a little bit easier to find those once it's time to uncomment them. I also like to add the major/minor version number to the comment to make it easier for whoever's doing the deprecation.

Also, we have have a replacement option that can be passed to the switch that helps with deprecating/disabling command flags. Although it seems, for some reason, that they can only be used for disabling rather than deprecations... Can you add commented out replacement lines for the --full and --shallow lines?

It should look something like this:

  def tap_args
    Homebrew::CLI::Parser.new do
      usage_banner "`tap` [<options>] [<user>`/`<repo>] [<URL>]"
      description <<~EOS
        Tap a formula repository.
        ...
      EOS
      switch "--full",
             description: "Convert a shallow clone to a full clone without untapping. Taps are only cloned as "\
                          "shallow clones if `--shallow` was originally passed."
             # TODO: (3.3) Uncomment and remove references to `args.full?`
             # replacement: false
      switch "--shallow",
             description: "Fetch tap as a shallow clone rather than a full clone. Useful for continuous integration."
             # TODO: (3.3) Uncomment and remove references to `args.shallow?`
             # replacement: false
      # ...
    end
  end

[cnnrmnn]

Also, we have have a replacement option that can be passed to the switch that helps with deprecating/disabling command flags. Although it seems, for some reason, that they can only be used for disabling rather than deprecations... Can you add commented out replacement lines for the --full and --shallow lines?

By disabling, do you mean removing the command altogether or deprecating it as a no-op?

[Rylan12]

By disabling, do you mean removing the command altogether or deprecating it as a no-op?

When we make backward incompatible changes, we do it in a three-part cycle:

1. Deprecation: this means that a warning is shown to users but the comand/method/flag still works (although if you have HOMEBREW_DEVELOPER set this becomes an error not a warning)
2. Disabling: this means that the comand/method/flag no longer works but a message is still shown
3. Removing the comand/method/flag completely

We advance to each level only on a major or minor release. So, when Homebrew 3.2.0 is released, we'll want these flags to be deprecated. 3.3.0 will have them disabled, and 3.4.0 will have them totally removed.

Right now it's fine for them to be a no-op because we haven't yet reached the 3.2.0 release mark. After that, though, they won't really be a no-op any more because if you try to pass one of them you'll either see an error with `brew tap --full` has been deprecated/disabled! (3.2-3.3) or invalid option: --foo (3.4+)

Does that help clarify?

[cnnrmnn]

Does that help clarify?

Yes, thanks!

[MikeMcQuaid]

@cnnrmnn Flaky tests, I'll rerun!

[cnnrmnn]

@cnnrmnn Flaky tests, I'll rerun!

Looks like they failed again. I'm unsure why.

[Bo98]

Rebase on master and it should work. The faulty test has been removed.

[Rylan12]

Thanks for the PR, @cnnrmnn! 🎉

Also, I noticed that you're a Yale student—I'll be joining you there next year!

[cnnrmnn]

Thanks for the PR, @cnnrmnn! 🎉

Also, I noticed that you're a Yale student—I'll be joining you there next year!

Awesome, welcome! Shoot me an email if you have any questions. :)

[Rylan12]

Thanks, will do! ❤️🐶