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

fix: remove empty glob from geographiclib #3495

Merged
merged 3 commits into from
Dec 28, 2024

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented Dec 27, 2024

  • as title

@wep21
Copy link
Contributor Author

wep21 commented Dec 27, 2024

@bazel-io skip_check unstable_url

@bazel-io bazel-io added the skip-url-stability-check Skip the URL stability check for the PR label Dec 27, 2024
@wep21
Copy link
Contributor Author

wep21 commented Dec 27, 2024

@fmeum @meteorcloudy please add presubmit auto run.

@wep21 wep21 mentioned this pull request Dec 27, 2024
@fmeum fmeum added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Dec 27, 2024
fmeum
fmeum previously approved these changes Dec 27, 2024
@fmeum fmeum enabled auto-merge (squash) December 27, 2024 13:28
@wep21
Copy link
Contributor Author

wep21 commented Dec 27, 2024

@fmeum The macos ci error seems to be irrelevant to this PR for me. How do you think about it?

@wep21 wep21 requested a review from kgreenek December 27, 2024 14:35
kgreenek
kgreenek previously approved these changes Dec 27, 2024
@wep21 wep21 requested a review from fmeum December 27, 2024 15:05
@fmeum
Copy link
Contributor

fmeum commented Dec 27, 2024

The failure persists. You could check with rules_foreign_cc whether this is a known issue. Maybe there is a way to have the ruleset use a hermetic Python provided by rules_python? That would also help users of your module that don't have Python installed locally.

@kgreenek
Copy link
Contributor

There are just a very small number of substitutions in this case in the Config.h file. You could consider just setting them manually in bzl and forgoing the python script that does the cmake parsing.

Signed-off-by: wep21 <[email protected]>
auto-merge was automatically disabled December 27, 2024 15:29

Head branch was pushed to by a user without write access

@bazel-io bazel-io dismissed stale reviews from fmeum and kgreenek December 27, 2024 15:29

Require module maintainers' approval for newly pushed changes.

@wep21 wep21 force-pushed the fix-empty-glob-geographiclib branch from 81a39a4 to ea97bfc Compare December 27, 2024 15:37
@wep21
Copy link
Contributor Author

wep21 commented Dec 27, 2024

@fmeum Setting default python version seems to solve the error, I'm not sure this is the proper python version though.

fmeum
fmeum previously approved these changes Dec 27, 2024
Copy link
Contributor

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

That's okay, the root module can override it.

kgreenek
kgreenek previously approved these changes Dec 27, 2024
modules/geographiclib/2.4.0.bcr.1/MODULE.bazel Outdated Show resolved Hide resolved
@bazel-io bazel-io dismissed stale reviews from fmeum and kgreenek December 28, 2024 02:02

Require module maintainers' approval for newly pushed changes.

@wep21 wep21 force-pushed the fix-empty-glob-geographiclib branch from a9bf856 to 5cebdbd Compare December 28, 2024 02:05
@wep21 wep21 requested review from fmeum and kgreenek December 28, 2024 02:08
Copy link
Contributor

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

Great, I'll keep this helper module in mind while reviewing other contributions!

@meteorcloudy FYI

@fmeum fmeum merged commit cb0bff3 into bazelbuild:main Dec 28, 2024
33 checks passed
@wep21 wep21 deleted the fix-empty-glob-geographiclib branch December 28, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants