Skip to content

changed most Settings pointers to references - #6298

Merged
danmar merged 1 commit into
cppcheck-opensource:mainfrom
firewave:ptr-ref-55xx-x
Apr 26, 2024
Merged

changed most Settings pointers to references#6298
danmar merged 1 commit into
cppcheck-opensource:mainfrom
firewave:ptr-ref-55xx-x

Conversation

@firewave

Copy link
Copy Markdown
Collaborator

No description provided.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

Requires #6297 to be merged first.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

It is quite disconcerting that none of the tests had to be adjusted...

@firewavefirewave added the merge-after-next-release Wait with merging this PR until after the next Release label Apr 16, 2024
@firewave

Copy link
Copy Markdown
CollaboratorAuthor

I think we should wait for the next release before we apply this so we leverage the daca diffs to see if this has some kind of feedback.

@firewavefirewave changed the title always provide settingschanged most Settings pointers to referencesApr 17, 2024
@firewave

firewave commented Apr 17, 2024

Copy link
Copy Markdown
CollaboratorAuthor

So I went all out and changed almost all Settings pointers to references. That enforces that it needs to be provided. It also no longer depends on that previous PR.

@firewave
firewave marked this pull request as ready for review April 17, 2024 09:20
@danmar

Copy link
Copy Markdown
Collaborator

I think we should wait for the next release before we apply this so we leverage the daca diffs to see if this has some kind of feedback.

👍

@firewavefirewave removed the merge-after-next-release Wait with merging this PR until after the next Release label Apr 22, 2024
@firewave

Copy link
Copy Markdown
CollaboratorAuthor

Would be good to merge this still early in the dev cycle so the fallout might be easier to differentiate from other changes.

@danmar
danmar merged commit b7360ce into cppcheck-opensource:mainApr 26, 2024
@firewave
firewave deleted the ptr-ref-55xx-x branch April 26, 2024 15:35
Sign up for freeto 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.

2 participants

@firewave@danmar