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

Improve generated C++ header files in the nmodl::ast namespace #1080

Merged
merged 7 commits into from
Sep 28, 2023

Conversation

tristan0x
Copy link
Member

This contribution provides the following change:

  1. Exclude some files from SonarSource analysis
  2. Fix a few smells spotted by SonarSource
  3. Improve the generated C++ headers in nmodl::ast namespace:
  • remove useless copies of std::shared_ptr and std::vector
  • use C++17 nested namespace definitions
  • the C++ headers have now an acceptable indentation, more readable by developers. I kept the compilation units intact since it is less essential.

You can find attached a diff of the <build>/src/ast directory

ast.patch

* Prefer passing by reference rather than copy when possible for `std::shared_ptr` and `std::vector`
* Use nested namespace definitions
* Improve indentation of generated headers for lisibility
@tristan0x tristan0x changed the title Tristan0x/sonar fixes SonarSource configuration improvements Sep 26, 2023
@tristan0x tristan0x marked this pull request as ready for review September 26, 2023 13:18
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (cda1d1e) 70.18% compared to head (fef71aa) 88.61%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1080       +/-   ##
===========================================
+ Coverage   70.18%   88.61%   +18.42%     
===========================================
  Files         188      169       -19     
  Lines       25627    12474    -13153     
===========================================
- Hits        17987    11054     -6933     
+ Misses       7640     1420     -6220     
Files Coverage Δ
src/codegen/codegen_cpp_visitor.cpp 85.54% <ø> (-0.01%) ⬇️
src/codegen/codegen_transform_visitor.cpp 100.00% <100.00%> (ø)

... and 23 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #156739 (:white_check_mark:) have been uploaded here!

Status and direct links:

@tristan0x tristan0x changed the title SonarSource configuration improvements Improve generated C++ header files in the nmodl::ast namespace Sep 28, 2023
@tristan0x tristan0x merged commit 2355a6e into master Sep 28, 2023
18 checks passed
@tristan0x tristan0x deleted the tristan0x/sonar-fixes branch September 28, 2023 16:05
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.

None yet

6 participants