Skip to content

Conversation

@firewave
Copy link
Collaborator

No description provided.

@firewave
Copy link
Collaborator Author

I was trying to make the object local instead of a class member but as it is being used by reportErr() that is unfortunately not possible.

Even I did not achieve that goal the usage checks got more explicit instead of relying on Settings::buildDir all over the code making things easier to read.

@firewave firewave marked this pull request as draft July 23, 2024 10:15
@firewave
Copy link
Collaborator Author

I think I might need to add some builddir-related tests first.

@firewave

This comment was marked as outdated.

@firewave firewave force-pushed the anainfo-2 branch 3 times, most recently from 2520d19 to 44d81e2 Compare November 25, 2024 12:03
@firewave firewave marked this pull request as ready for review November 25, 2024 12:14
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


if (!mSettings.buildDir.empty()) {
mAnalyzerInformation.close();
if (mAnalyzerInformation) {
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: I guess we don't need the condition. We can just reset directly and let it check internally..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense - but I intentionally kept it intact for now. The future refactorings might get rid of it.

@firewave firewave merged commit cbc1661 into danmar:main Dec 3, 2024
60 checks passed
@firewave firewave deleted the anainfo-2 branch December 3, 2024 14:34
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.

2 participants