Skip to content

[doNOTmerge] check if filesize increase depends on comment title #18487

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

Closed
wants to merge 7 commits into from

Conversation

ferdymercury
Copy link
Contributor

This Pull request:

Changes or fixes:

See #18459 (comment)

@ferdymercury ferdymercury requested a review from pcanal April 24, 2025 08:25
@ferdymercury
Copy link
Contributor Author

ferdymercury commented Apr 24, 2025

No, it does not depend on the number of functions.
Trying now removing things.
Checking now if the const intmight be the problem.

Copy link

github-actions bot commented Apr 24, 2025

Test Results

     6 files       6 suites   22h 0m 36s ⏱️
 2 694 tests  2 694 ✅ 0 💤 0 ❌
16 042 runs  16 042 ✅ 0 💤 0 ❌

Results for commit 27ae438.

♻️ This comment has been updated with latest results.

@ferdymercury ferdymercury changed the title [doNOTmerge] check if filesize increase is proportional to number of functions [doNOTmerge] check if filesize increase depends on const int parameter Apr 24, 2025
@ferdymercury ferdymercury reopened this Apr 24, 2025
@ferdymercury
Copy link
Contributor Author

@linev @pcanal this little change is the one that leads to a change in the compressed file size:

27ae438

@linev
Copy link
Member

linev commented Apr 24, 2025

@pcanal

Is it by chance class checksum which changes after changing in class methods?

@ferdymercury
Copy link
Contributor Author

ferdymercury commented Apr 24, 2025

@pcanal

Is it by chance class checksum which changes after changing in class methods?

Note that if I add many other functions the filesize does not change. Only if a parameter "const int" instead of "int" appears.

@linev
Copy link
Member

linev commented Apr 24, 2025

For me checksum remains.

@ferdymercury
Copy link
Contributor Author

EDIT: False alarm, it seems it's not related to the const int.
Now trying with the modified comment descriptor.

@ferdymercury ferdymercury changed the title [doNOTmerge] check if filesize increase depends on const int parameter [doNOTmerge] check if filesize increase depends on comment title Apr 24, 2025
@linev
Copy link
Member

linev commented Apr 24, 2025

@ferdymercury
Yes, changed comments can be a reason. They stored with streamer infos in the file.
Probably just modify your original PR and do not touch comments there .

@ferdymercury
Copy link
Contributor Author

ferdymercury commented Apr 24, 2025

@ferdymercury Yes, changed comments can be a reason. They stored with streamer infos in the file. Probably just modify your original PR and do not touch comments there .

But I need there MENU comments anyway, so I will need to still modify the roottests ? Or only comments of members are stored but not comments of functions ?

@linev
Copy link
Member

linev commented Apr 24, 2025

Or only comments of members are stored but not comments of functions ?

Yes, only persistent members comments stored in the file.
Class methods are not listed in streamer infos.

@ferdymercury ferdymercury deleted the filesize2 branch April 24, 2025 12:46
@linev
Copy link
Member

linev commented Apr 24, 2025

You can check here that is stored in streamer infos of hsimple.root file.

@pcanal
Copy link
Member

pcanal commented Apr 24, 2025

Is it by chance class checksum which changes after changing in class methods?

The CheckSum should only change if you change the type (or name) of any persistent data member. The comment should have no effect (hopefully)

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.

3 participants