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

archive: delete #12130

Merged
merged 1 commit into from
Oct 1, 2021
Merged

archive: delete #12130

merged 1 commit into from
Oct 1, 2021

Conversation

FnControlOption
Copy link
Contributor

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

These lines in Archive#upload_bottles:

      directory = if formula_tap_repo
        "bottles-#{tap.repo}"
      else
        "bottles"
      end

came from this method in Utils::Bottles::Bintray (which was deleted in b914411):

      def self.repository(tap = nil)
        if tap.nil? || tap.core_tap?
          "bottles"
        else
          "bottles-#{tap.repo}"
        end
      end

However, upload_bottles does not have a tap parameter, and tap.repo should be replaced with formula_tap_repo.

@MikeMcQuaid
Copy link
Member

Thanks @FnControlOption! Are you using this to upload to the Internet Archive or did you just notice it? I'm wondering if this code should be removed if it was broken and no-one noticed.

@FnControlOption
Copy link
Contributor Author

Not using it. Found this with brew typecheck. I agree that this can safely be deleted if no one has opened issue about this from when b914411 was pushed to master to now. However, I don't know exactly how this works, so I'll defer to you.

@MikeMcQuaid
Copy link
Member

@FnControlOption I reckon just open a pull request to remove this. Thanks!

@MikeMcQuaid
Copy link
Member

(or update this one to do so, even better)

@FnControlOption FnControlOption changed the title archive: fix reference to formula tap repo archive: delete Sep 30, 2021
@MikeMcQuaid MikeMcQuaid merged commit 04b54df into Homebrew:master Oct 1, 2021
@MikeMcQuaid
Copy link
Member

Thanks again @FnControlOption!

@github-actions github-actions bot added the outdated PR was locked due to age label Nov 1, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 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

2 participants