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

[question] Support for Code Quality Checks #14720

Closed
1 task done
jsallay opened this issue Sep 12, 2023 · 7 comments
Closed
1 task done

[question] Support for Code Quality Checks #14720

jsallay opened this issue Sep 12, 2023 · 7 comments
Assignees

Comments

@jsallay
Copy link
Contributor

jsallay commented Sep 12, 2023

What is your question?

What would be the best way to handle code quality tools in conan? For example, I may want to run a linter such as cpplint or lcov for test coverage when I build my package.

These tools will produce a report and may have a pass fail condition. I don't presently see a good way to integrate them into conan. I would imagine that the tools would be listed as build_requirements in the conanfile. I can handle the pass/fail aspect pretty easily.

I see two problems though:

  1. I produce several versions of the package based on different settings. I don't need to run the coverage on every version. It would be nice to only enable it sometimes, kind of like unit tests.
  2. I don't have an elegant way to handle the reports. They could be included as part of the final package, but that seems like an odd choice and wouldn't be available if I skipped the checks as in (1). If they aren't in the final package, then I don't have a good way to extract them.

Is conan not really designed around this use case, am I missing something, or should this be a feature request? I presently doing all of my linting outside of conan, but I think it would be really helpful to encapsulate it all inside of the conanfile.

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded
Copy link
Member

Hi @jsallay

Thanks for your question!

I produce several versions of the package based on different settings. I don't need to run the coverage on every version. It would be nice to only enable it sometimes, kind of like unit tests.

Yes, I think the approach for this would depend if the instrumentation creates or not a different binary (not only side artifacts, but the actual library/binary is instrumented and could be slower, etc):

  • If the binary is different, then adding a custom setting or an option is necessary, to model the different binary.
    • Use a custom setting, probably a subsetting of the compiler, if it is intended to instrument the majority of package builds, and specially if instrumented builds need other instrumented builds (like might be the case of sanitizers)
    • Use options per-package, if the instrumentation can be different for different packages, do not depend on the other packages instrumentation.
  • If the binary is not different, just adding extra steps in the build, that want to be controlled by user inputs, then a custom user.myteam:myconf conf would be better

I don't have an elegant way to handle the reports. They could be included as part of the final package, but that seems like an odd choice and wouldn't be available if I skipped the checks as in (1). If they aren't in the final package, then I don't have a good way to extract them.

This might be a good case for the new "metadata files" feature (merged, but not documented or released yet). It allows to store extra files in recipes and packages, but those files are not downloaded at every package install, only on-demand to collect or inspect them if necessary. The plan is to launch this soon, maybe after 2.0.11.

You might also capture the results from a hook, and upload them automatically or something like that, if you need a different storage. If you want your reports to be stored together with your packages in the server, then the "metadata files" might be a better solution.

Please let me know if this helps, cheers!

@jsallay
Copy link
Contributor Author

jsallay commented Sep 13, 2023

The metadata files sounds interesting. Can you link me to a PR or issue?

@memsharded
Copy link
Member

Sure! It will be these PRs (already merged):

And this one pending for next release:

You can probably check and try with the already released functionality, last PR is not critical to give it a try

Just beware that if uploading metadata to servers, Conan 1.X old versions might fetch it. Latest Conan 1.X has already provisions to ignore the metadata and not download it.

@jsallay
Copy link
Contributor Author

jsallay commented Sep 13, 2023

I just want to double check how I would use this. Would I put files in the metadata folder during the build or would I copy them in during the package step?

@memsharded
Copy link
Member

I think the final result should be very similar, but as build() tends to be more complex and output-cluttering, I'd probably do it in the package() method, as it can be a bit more evident which metadata is being finally stored.

@jsallay
Copy link
Contributor Author

jsallay commented Sep 13, 2023

Thanks. This looks like exactly what I needed. I'll go ahead and close the issue.

@jsallay jsallay closed this as completed Sep 13, 2023
@memsharded
Copy link
Member

Good! Feedback over the metadata feature would be very appreciated. We will also make some announcement when it is documented and officially launched. Many thanks!

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

No branches or pull requests

2 participants