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

Feature/3338/improve parser suggestion 3 #3414

Merged
merged 20 commits into from Nov 24, 2023

Conversation

fritschldwg
Copy link
Collaborator

@fritschldwg fritschldwg commented Nov 20, 2023

Feature/3338/improve parser suggestion 3

Closes: #3338

Description

  • Enhanced logging: Now logs the absolute file paths of the output files.
  • Bug fix: Resolved an issue in the CSVExporter that previously prevented the exporting of multiple projects.
  • Test coverage: Added tests for the CSVExporter.

Definition of Done

A PR is only ready for merge once all the following acceptance criteria are fulfilled:

  • Changes have been manually tested
  • All TODOs related to this PR have been closed
  • There are automated tests for newly written code and bug fixes
  • All bugs discovered while working on this PR have been submitted as issues (if not already an open issue)
  • Documentation (GH-pages, analysis/visualisation READMEs, parser READMEs, --help, etc.) has been updated (almost always necessary except for bug fixes)
  • CHANGELOG.md has been updated

@fritschldwg
Copy link
Collaborator Author

fritschldwg commented Nov 20, 2023

I'm uncertain about whether to include unit tests to validate the logging behavior.

Pros of adding unit tests:

  • Offers explicit verification that logging functions correctly for each parser.

Cons of adding unit tests:

  • Does not assess business logic; issues related to incorrect file creation should be caught by other tests.
  • Introduces potential brittleness to the tests.

@fritschldwg fritschldwg reopened this Nov 20, 2023
@fritschldwg fritschldwg force-pushed the feature/3338/improve-parser-suggestion-3 branch from 06d08ce to 4c17a9f Compare November 21, 2023 11:04
Copy link
Collaborator

@ce-bo ce-bo left a comment

Choose a reason for hiding this comment

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

LGTM in general. I added some suggestions on how to improve a little bit.

@fritschldwg fritschldwg force-pushed the feature/3338/improve-parser-suggestion-3 branch 3 times, most recently from ebf0aa2 to d99aa61 Compare November 23, 2023 19:38
@fritschldwg fritschldwg force-pushed the feature/3338/improve-parser-suggestion-3 branch from d99aa61 to 28624ea Compare November 24, 2023 08:28
Copy link

sonarcloud bot commented Nov 24, 2023

[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link

sonarcloud bot commented Nov 24, 2023

[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

93.2% 93.2% Coverage
0.0% 0.0% Duplication

@fritschldwg fritschldwg merged commit 48e47f7 into main Nov 24, 2023
7 checks passed
@fritschldwg fritschldwg deleted the feature/3338/improve-parser-suggestion-3 branch November 24, 2023 08:44
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.

Parser suggestions - Improvements
3 participants