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

Avoid using cmake glob vars if we are a subproject v2 #1459

Merged

Conversation

nkh-lab
Copy link
Contributor

@nkh-lab nkh-lab commented Jan 11, 2023

If jsoncpp is a subproject (like a git submodule), setting the global cmake variables affect the entire project (changes the structure of the output folders) and these changes prevent it.

@nkh-lab
Copy link
Contributor Author

nkh-lab commented Jan 11, 2023

This is another approach of how to fix given issue, previous proposed by @mknaranja is PR-1453, issue discussion is here Issue-1451.

@atrelinski
Copy link

IMO it's a very good and necessary fix!

@BillyDonahue
Could You merge it?

@BillyDonahue BillyDonahue self-requested a review January 16, 2023 06:33
@BillyDonahue
Copy link
Contributor

Thanks for doing this!

@BillyDonahue
Copy link
Contributor

Anyone know what's up with Travis CI? It seems to be blocked.

@eli-schwartz
Copy link

Anyone know what's up with Travis CI? It seems to be blocked.

Maybe you've exhausted your current allowance of sponsored "build minutes for OSS" and have to apply for the next batch?

@BillyDonahue
Copy link
Contributor

Anyone know what's up with Travis CI? It seems to be blocked.

Maybe you've exhausted your current allowance of sponsored "build minutes for OSS" and have to apply for the next batch?

Yes that's what's up.

Builds have been temporarily disabled for private and public repositories due to a negative credit balance.

@cdunn2001 is the contact for that.
We could wait out the month and rebuild when we get the OSS credits refresh.

@baylesj
Copy link
Contributor

baylesj commented Jan 22, 2023

Anyone know what's up with Travis CI? It seems to be blocked.

Maybe you've exhausted your current allowance of sponsored "build minutes for OSS" and have to apply for the next batch?

Yes that's what's up.

Builds have been temporarily disabled for private and public repositories due to a negative credit balance.

@cdunn2001 is the contact for that. We could wait out the month and rebuild when we get the OSS credits refresh.

Waiting out the month seems reasonable. It's a bummer that the open screen credits are shared between all of the @open-source-parsers repositories, considering we haven't really had any code checked into JsonCpp in over six months.

@nkh-lab
Copy link
Contributor Author

nkh-lab commented May 29, 2023

Looks like the PR is stuck...

@quic-egmc
Copy link

@nkh-lab @BillyDonahue can you please fix this PR (CI) so it can be merged?

@BillyDonahue
Copy link
Contributor

ok. it's a good change. We just can't get CI to validate it.
We can just YOLO it and be ready to revert if necessary I guess.

If jsoncpp is a subproject (like a git submodule), setting the
global cmake variables affect the entire project (changes the
structure of the output folders) and these changes prevent it.
@BillyDonahue BillyDonahue force-pushed the Avoid-using-cmake-glob-vars branch from 0462d0b to e3a74a7 Compare June 27, 2023 14:40
@BillyDonahue BillyDonahue merged commit 69098a1 into open-source-parsers:master Jun 27, 2023
nkh-lab added a commit to nkh-lab/usb-relay-module that referenced this pull request Jul 11, 2023
Since PR: open-source-parsers/jsoncpp#1459
was merged we can use origin jsoncpp now.

Since PR open-source-parsers/jsoncpp#1459
has been merged, we can now use origin jsoncpp project.
nkh-lab added a commit to nkh-lab/usb-relay-module that referenced this pull request Jul 11, 2023
Since PR open-source-parsers/jsoncpp#1459
has been merged, we can now use origin jsoncpp project.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants