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

refs #352 - generate output error on unknown DUI::std value #312

Merged
merged 1 commit into from
May 22, 2024
Merged

refs #352 - generate output error on unknown DUI::std value #312

merged 1 commit into from
May 22, 2024

Conversation

firewave
Copy link
Collaborator

If the value is unknown we are silently setting an invalid/unexcepted value for the predefined macros which might lead to other issues.

danmar
danmar reviewed Sep 11, 2023
View reviewed changes
simplecpp.cpp Outdated Show resolved Hide resolved
@danmar
Copy link
Owner

danmar commented Sep 11, 2023

I guess a simplecpp issue wouldn't hurt also

@firewave firewave marked this pull request as draft September 11, 2023 18:11
@firewave
Copy link
Collaborator Author

I guess a simplecpp issue wouldn't hurt also

Good point. Tickets for behavior changes.

@firewave
Copy link
Collaborator Author

I adjusted the code so it uses the OutputList instead of throwing an exception.

Will file a ticket about this later.

@firewave firewave force-pushed the unk-std branch 2 times, most recently from 9e41599 to ffba6e5 Compare April 24, 2024 18:52
@firewave firewave mentioned this pull request May 3, 2024
Merged
@firewave firewave changed the title bail out on unknown DUI::std value refs #352 - bail out on unknown DUI::std value May 6, 2024
@firewave
Copy link
Collaborator Author

firewave commented May 6, 2024

Will file a ticket about this later.

Done.

@firewave firewave marked this pull request as ready for review May 6, 2024 14:33
danmar
danmar approved these changes May 21, 2024
View reviewed changes
Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

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

lgtm, should I merge?

@firewave
Copy link
Collaborator Author

I thought we wanted to make a release with the current state first so we can get this repo and Cppcheck in sync again.

Afterwards feel free to merge anything I made ready for review and you approve of.

@firewave firewave changed the title refs #352 - bail out on unknown DUI::std value refs #352 - generate output error on unknown DUI::std value May 21, 2024
@danmar
Copy link
Owner

danmar commented May 22, 2024

I thought we wanted to make a release with the current state first so we can get this repo and Cppcheck in sync again.
Afterwards feel free to merge anything I made ready for review and you approve of.

Sorry.. I have tagged 1.1.3 now. I tagged the old commit e040047

Please feel free to sync cppcheck.

@danmar danmar merged commit 59f376c into danmar:master May 22, 2024
14 checks passed
@firewave firewave deleted the unk-std branch May 22, 2024 09:02
@firewave
Copy link
Collaborator Author

@danmar I changed the title of the PR because we do not actually bail out anymore. Strangely your merge still had the old title. I have no idea how this is even possible!?

@firewave firewave mentioned this pull request May 22, 2024
Open
@firewave
Copy link
Collaborator Author

Actually it was merge with the original title from when I created this PR. I changed the title multiple times.

I have no seen this in the Cppcheck repo so far but there I usually merge the PRs myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danmar danmar danmar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@firewave @danmar