-
Notifications
You must be signed in to change notification settings - Fork 185
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
Add 'brotli' package #604
Conversation
docs/packages/pkg/brotli.rst
Outdated
- `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>`__) |
There was a problem hiding this comment.
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
cmake/projects/brotli/hunter.cmake
Outdated
VERSION | ||
1.0.9-p0 | ||
URL | ||
"https://github.com/drodin/brotli/archive/refs/heads/hunter-1.0.9.zip" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, fixed @drodin
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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
examples/brotli/CMakeLists.txt
Outdated
# Copyright (c) 2016-2020, Rahul Sheth, Ruslan Baratov | ||
# All rights reserved. | ||
|
||
cmake_minimum_required(VERSION 3.2) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
cmake_minimum_required(VERSION 3.2) | |
cmake_minimum_required(VERSION 3.5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
a237b23
to
c521429
Compare
@rbsheth I've updated URLs to point to cpp-pm/brotli. Can we merge this? |
There was a problem hiding this 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/CMakeLists.txt
Outdated
# Copyright (c) 2016-2020, Rahul Sheth, Ruslan Baratov | ||
# All rights reserved. | ||
|
||
cmake_minimum_required(VERSION 3.2) |
There was a problem hiding this comment.
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
cmake_minimum_required(VERSION 3.2) | |
cmake_minimum_required(VERSION 3.5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good, merging
needs a fork of https://github.com/google/brotli in cpp-pm org
will update to correct URL after
step by step carefully. [Yes]