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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

jsontoolkit 2.0.0 (new formula) #171151

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

jsontoolkit 2.0.0 (new formula) #171151

wants to merge 6 commits into from

Conversation

tony-go
Copy link

@tony-go tony-go commented May 8, 2024

Hi there 馃憢馃徏

We'd like to add https://github.com/sourcemeta/jsontoolkit to homebrew :)

I followed the guide. I hope I did not forget anything.

Cheers.

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@tony-go
jsontoolkit 2.0.0
add4949 
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
@github-actions github-actions bot added the new formula PR adds a new formula to Homebrew/homebrew-core label May 8, 2024
@tony-go tony-go mentioned this pull request May 8, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 8, 2024

Thanks for contributing to Homebrew! 馃帀 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request.

@carlocab carlocab changed the title Add jsontoolkit 2.0.0 jsontoolkit 2.0.0 (new formula) May 8, 2024
carlocab
carlocab reviewed May 8, 2024
View reviewed changes
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
@carlocab carlocab added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label May 8, 2024
@tony-go
Copy link
Author

tony-go commented May 8, 2024 •
edited

I just saw this from Linux CI:

audit --formula jsontoolkit --online --new output
  jsontoolkit
    * GitHub repository not notable enough (<30 forks, <30 watchers and <75 stars)
  Error: 1 problem in 1 formula detected.

Something I did not get locally. 馃

@tony-go
Copy link
Author

tony-go commented May 8, 2024

Thanks for your review @carlocab :)

@tony-go
fix: clean cmake args and enable shared libs
0b52034 
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label May 8, 2024
carlocab
carlocab reviewed May 8, 2024
View reviewed changes
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
@chenrui333 chenrui333 added the notability Project is not notable enough for inclusion label May 8, 2024
jviotti
jviotti reviewed May 8, 2024
View reviewed changes
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
jviotti
jviotti reviewed May 8, 2024
View reviewed changes
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
@tony-go
fix: use c++20 and set rpath at cmake level
6e2458a 
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
chenrui333
chenrui333 reviewed May 8, 2024
View reviewed changes
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
@tony-go
fix: use cmake file for testing
106b6b0 
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
jviotti
jviotti reviewed May 13, 2024
View reviewed changes
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
jviotti
jviotti reviewed May 13, 2024
View reviewed changes
Formula/j/jsontoolkit.rb

int main() {
// JSON
std::istringstream input{R"([ { "foo": 1 }, { "bar": 2 } ])"};
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want, note you can pass the text directly to parse()

Copy link
Author

Choose a reason for hiding this comment

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

Yeah for this comment and this #171151 (comment)

let see when CI is green and Notability issue resolved.

jviotti
jviotti reviewed May 13, 2024
View reviewed changes
Formula/j/jsontoolkit.rb
"type": "string"
})JSON");

const auto schema_template{sourcemeta::jsontoolkit::compile(
Copy link
Contributor

Choose a reason for hiding this comment

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

The schema compiler only officially supports Draft 4 for now, not 2020-12. Can you set a Draft 4 metaschema instead?

Copy link
Member

Choose a reason for hiding this comment

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

cc @tony-go

carlocab
carlocab reviewed May 14, 2024
View reviewed changes
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
@tony-go
refactor: switch back to basic cc compiler
f098306 
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
@tony-go tony-go requested a review from carlocab May 15, 2024 08:52
jviotti
jviotti reviewed May 15, 2024
View reviewed changes
Formula/j/jsontoolkit.rb Outdated Show resolved Hide resolved
@tony-go
fix: code indent
c187d22 
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
@tony-go
Copy link
Author

tony-go commented May 16, 2024

hey @chenrui333 @carlocab 馃憢馃徏

How does notability issues are handled, generally speaking?

@chenrui333
Copy link
Member

looks like there are some outstanding comments?

@chenrui333
Copy link
Member

@tony-go also can you squash the commits and add the formula into autobump.txt? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jviotti jviotti jviotti left review comments

@chenrui333 chenrui333 Awaiting requested review from chenrui333

@carlocab carlocab Awaiting requested review from carlocab

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
autosquash Automatically squash pull request commits according to Homebrew style. CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. new formula PR adds a new formula to Homebrew/homebrew-core notability Project is not notable enough for inclusion
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tony-go @chenrui333 @jviotti @carlocab