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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for CMakeDeps generator being duplicated when using a conanfile.py #584

Merged
merged 10 commits into from
Nov 14, 2023
Merged

Fix for CMakeDeps generator being duplicated when using a conanfile.py #584

merged 10 commits into from
Nov 14, 2023

Conversation

juansblanco
Copy link
Contributor

@juansblanco juansblanco commented Nov 7, 2023 •
edited
Loading

Fix for #540

  • Issue: Fixed an error when CMakeDeps generator was declared twice using the conanfile.py
  • Fix: Now if there's a conanfile.py it won't be passed as argument in the conan-provider.cmake to avoid this issue.
  • Added Warning when there's a conanfile with no declared generator
  • Added tests to cover possible errors with generators

memsharded
memsharded reviewed Nov 7, 2023
View reviewed changes
Copy link
Member

@memsharded memsharded 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

conan_provider.cmake Show resolved Hide resolved
@juansblanco
Copy link
Contributor Author

I added warnings for both conanfile.txt and conanfile.py.
I'm not sure if it's mandatory in conanfile.py or if it can work without having one. It could be changed to error instead of warning if needed.
Also there can be the case where it finds the word in a comment without it being declared.

@jcar87 jcar87 force-pushed the js/bugfix/conan-provider-generators branch from e4d5f9a to 0f6f28a Compare November 9, 2023 10:24
jcar87
jcar87 reviewed Nov 9, 2023
View reviewed changes
tests/test_smoke.py Outdated Show resolved Hide resolved
jcar87
jcar87 reviewed Nov 9, 2023
View reviewed changes
tests/test_smoke.py Outdated Show resolved Hide resolved
jcar87
jcar87 reviewed Nov 9, 2023
View reviewed changes
Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

Good job! I've rebased the branch on top of the most recent changes on develop2 -

Added a few comments, but nothing major

jcar87
jcar87 reviewed Nov 9, 2023
View reviewed changes
tests/test_smoke.py Outdated
@@ -40,6 +40,8 @@ def run(cmd, check=True):
@contextmanager
def chdir(folder):
Copy link
Contributor

Choose a reason for hiding this comment

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

I need to remove this completely (will do on a separate PR) as it is no longer used

juansblanco
juansblanco commented Nov 13, 2023
View reviewed changes
tests/test_smoke.py Outdated Show resolved Hide resolved
@jcar87 jcar87 marked this pull request as ready for review November 14, 2023 09:25
@jcar87 jcar87 merged commit bd279af into conan-io:develop2 Nov 14, 2023
5 checks passed
@juansblanco juansblanco deleted the js/bugfix/conan-provider-generators branch November 14, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@memsharded memsharded memsharded left review comments

@jcar87 jcar87 jcar87 approved these changes

Assignees

@jcar87 jcar87

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@juansblanco @jcar87 @memsharded