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

Add 'brotli' package #604

Merged
merged 1 commit into from
May 15, 2024
Merged

Add 'brotli' package #604

merged 1 commit into from
May 15, 2024

Conversation

drodin
Copy link
Member

@drodin drodin commented Sep 6, 2022

needs a fork of https://github.com/google/brotli in cpp-pm org
will update to correct URL after

  • I've followed this guide
    step by step carefully. [Yes]

examples/brotli/boo.cpp Show resolved Hide resolved
- `Official <https://github.com/google/brotli>`__
- `Hunterized <https://github.com/cpp-pm/brotli>`__
- `Example <https://github.com/cpp-pm/hunter/blob/master/examples/brotli/CMakeLists.txt>`__
- Added by `drodin <https://github.com/drodin>`__ (`pr-N <https://github.com/cpp-pm/hunter/pull/N>`__)

Choose a reason for hiding this comment

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

please update the PR number

VERSION
1.0.9-p0
URL
"https://github.com/drodin/brotli/archive/refs/heads/hunter-1.0.9.zip"

Choose a reason for hiding this comment

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

@rbsheth please create a fork of https://github.com/google/brotli at https://github.com/cpp-pm/brotli for @drodin to access and use

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@rbsheth @NeroBurner I don't have access rights to this repo.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, fixed @drodin

Copy link
Member Author

Choose a reason for hiding this comment

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

@rbsheth Still can't push a new branch:

remote: error: GH006: Protected branch update failed for refs/heads/hunter-1.0.9.
remote: error: At least 1 approving review is required by reviewers with write access. You're not authorized to push to this branch. Visit https://docs.github.com/articles/about-protected-branches/ for more information.
To github.com:cpp-pm/brotli.git
 ! [remote rejected] hunter-1.0.9 -> hunter-1.0.9 (protected branch hook declined)

Copy link
Member

Choose a reason for hiding this comment

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

Wow just saw this, sorry @drodin. You can't push to a hunter* branch directly after it has been created, so try pushing to another branch name and creating a PR for review.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @rbsheth. I want to revive this PR. There was never before a branch hunter* created in this repo. I'm sure that I was able to push to a new hunter* branch in other repos. I was able to push to brunch pr.hunter-1.0.9. What should I choose as a target for a PR if there is no hunter* branch? Can't rename pr.hunter-1.0.9 branch to hunter-1.0.9 as well...

Copy link
Member

Choose a reason for hiding this comment

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

Hi @drodin, not sure what happened so I made a hunter-1.0.9 branch for you https://github.com/cpp-pm/brotli/tree/hunter-1.0.9

@NeroBurner NeroBurner added the package:new New package label Sep 9, 2022
# Copyright (c) 2016-2020, Rahul Sheth, Ruslan Baratov
# All rights reserved.

cmake_minimum_required(VERSION 3.2)
Copy link

Choose a reason for hiding this comment

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

This should probably be replaced with 3.5.

Choose a reason for hiding this comment

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

Yes please use 3.5 otherwise we get deprecation warnings while running this example

Suggested change
cmake_minimum_required(VERSION 3.2)
cmake_minimum_required(VERSION 3.5)

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@drodin drodin force-pushed the pr.brotli branch 2 times, most recently from a237b23 to c521429 Compare April 26, 2024 01:21
@drodin
Copy link
Member Author

drodin commented Apr 26, 2024

@rbsheth I've updated URLs to point to cpp-pm/brotli. Can we merge this?

Copy link

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

Just the cake version update and then we're good to go

examples/brotli/boo.cpp Show resolved Hide resolved
# Copyright (c) 2016-2020, Rahul Sheth, Ruslan Baratov
# All rights reserved.

cmake_minimum_required(VERSION 3.2)

Choose a reason for hiding this comment

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

Yes please use 3.5 otherwise we get deprecation warnings while running this example

Suggested change
cmake_minimum_required(VERSION 3.2)
cmake_minimum_required(VERSION 3.5)

Copy link

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

looking good, merging

@NeroBurner NeroBurner merged commit e31fe32 into cpp-pm:master May 15, 2024
19 of 20 checks passed
@NeroBurner
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:new New package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants