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

zc 0.14.0 (new cask) #169269

Closed
wants to merge 3 commits into from
Closed

zc 0.14.0 (new cask) #169269

wants to merge 3 commits into from

Conversation

zectdev
Copy link

@zectdev zectdev commented Mar 17, 2024

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

In the following questions <cask> is the token of the cask you're submitting.

After making any changes to a cask, existing or new, verify:

Additionally, if adding a new cask:

  • Named the cask according to the token reference.
  • Checked the cask was not already refused.
  • Checked the cask is submitted to the correct repo.
  • brew audit --cask --new <cask> worked successfully.
  • HOMEBREW_NO_INSTALL_FROM_API=1 brew install --cask <cask> worked successfully.
  • brew uninstall --cask <cask> worked successfully.

@BrewTestBot BrewTestBot added new cask missing zap Cask is missing a zap stanza, please add one. labels Mar 17, 2024
@daeho-ro
Copy link
Member

It seems that the binary should go to the homebrew core first.

@bevanjkay
Copy link
Member

If the binary is built from a closed-source then homebrew-cask is the appropriate place for this to be submitted.

Aside to this, I am not convinced that the app carries enough notability to be included at this stage. Self-submission (by the vendor) is often an initial sign that there is not demand for the cask to be included in homebrew-cask. Additionally, a lack of SSL certificate on the homepage and download links points to the project not having a required level of maturity to be included as is.

It may be better to begin by offering this in your own tap.

@zectdev
Copy link
Author

zectdev commented Mar 18, 2024

This is indeed built closed-source. We use a wildcard SSL cert https://releases.zectonal.com/ and https://www.zectonal.com/

@bevanjkay
Copy link
Member

Why not use those links in the cask file?

@zectdev
Copy link
Author

zectdev commented Mar 18, 2024

I didn't think of it since this is the first attempt, but I could easily do that...what is the process to handle this PR if I go back and use an HTTPS URL? Should I resubmit a new PR?

@bevanjkay
Copy link
Member

You can push additional commits to this PR. We don't currently require commits to be squashed in homebrew-cask either.

@zectdev
Copy link
Author

zectdev commented Mar 18, 2024

Great news...I will update and push the new commits to this PR - thank you for the feedback.

@zectdev
Copy link
Author

zectdev commented Mar 18, 2024

I updated with https that uses our SSL cert

Casks/z/zc.rb Outdated

livecheck do
url "http://zectonal-releases.s3-website-us-east-1.amazonaws.com/zc/releases/download/index.html"
url "https://docs.zectonal.com/zc/releases/download/index.html"
strategy :page_match
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
strategy :page_match

The strategy can be removed as page_match is the default.

Casks/z/zc.rb Outdated
strategy :page_match
# regex(%r{href=.*?\"v(\d+(?:\.\d+)*)\/index\.html\"}i)
regex(%r{href=.*?"http://zectonal-releases\.s3-website-us-east-1\.amazonaws\.com/zc/releases/download/v(\d+(?:\.\d+)*).*}i)
regex(/href=.*v(\d+(?:\.\d+)*).*/i)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
regex(/href=.*v(\d+(?:\.\d+)*).*/i)
regex(%r{href=.*?download/v?(\d+(?:\.\d+)+)/index.html}i)

Copy link
Author

Choose a reason for hiding this comment

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

thank you for this feedback. I incorporated into the latest commit below.

@chenrui333 chenrui333 changed the title initial commit zc 0.14.0 zc 0.14.0 (new cask) Mar 19, 2024
@razvanazamfirei
Copy link
Contributor

If no zap is needed for this cask, we should add # No zap stanza required.

Regardless, I agree with @bevanjkay that this does not meet our notability requirements.

Thank you for the PR, @zectdev and we really appreciate your contribution! Unfortunately, we're going to pass on this for now.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
missing zap Cask is missing a zap stanza, please add one. new cask outdated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants