Skip to content

Conversation

@kamahori
Copy link
Contributor

@kamahori kamahori commented Mar 2, 2020

I added unit test TTree.cxx which checks default compression settings used for TFile.

@kamahori kamahori requested review from oshadura and pcanal as code owners March 2, 2020 13:38
@phsft-bot
Copy link

Can one of the admins verify this patch?

@oshadura
Copy link
Collaborator

oshadura commented Mar 2, 2020

@pcanal just FYI, it is a GSOC exersize PR :)

@oshadura
Copy link
Collaborator

oshadura commented Mar 2, 2020

@kamahori Can you add a proper commit message explaining what this code is doing? (it could be helpful https://chris.beams.io/posts/git-commit/)

@oshadura
Copy link
Collaborator

oshadura commented Mar 2, 2020

@phsft-bot build!

@phsft-bot
Copy link

Starting build on ROOT-performance-centos7-multicore/default, ROOT-fedora27/noimt, ROOT-fedora29/python3, ROOT-fedora30/cxx14, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu18.04-i386/cxx14, mac1014/cxx17, windows10/cxx14
How to customize builds

@oshadura oshadura self-assigned this Mar 2, 2020
@phsft-bot
Copy link

Build failed on windows10/cxx14.
See console output.

@kamahori kamahori changed the title [TTree] added TTree.cxx [TTree] Add TTree.cxx Mar 3, 2020
@kamahori
Copy link
Contributor Author

kamahori commented Mar 3, 2020

How can I see the console output?
I cannot create my account on CERN Accelerating science page.

@oshadura
Copy link
Collaborator

oshadura commented Mar 3, 2020

@kamahori Jenkins results of tests are not available for people without CERN account, I am sorry for this. Windows failure is not connected with your test, but is actually infrastructure configuration issue (you need to fork https://github.com/root-project/roottest and it will be gone).

@kamahori
Copy link
Contributor Author

kamahori commented Mar 3, 2020

@phsft-bot build

@oshadura
Copy link
Collaborator

oshadura commented Mar 3, 2020

@phsft-bot build just on windows10/cxx14

@phsft-bot
Copy link

Starting build on windows10/cxx14
How to customize builds

@kamahori
Copy link
Contributor Author

kamahori commented Mar 3, 2020

Sorry, I'm not sure how to remove merge message.

@oshadura oshadura changed the title [TTree] Add TTree.cxx [TTree] Add compression algorithm test for TTree Mar 3, 2020
@oshadura oshadura changed the title [TTree] Add compression algorithm test for TTree [TTree] Add compression algorithm test for TFile/TTree Mar 3, 2020
@oshadura
Copy link
Collaborator

@pcanal asked if you can rename file from TTree.cxx to testTTreeCompression.cxx or something similar not to confuse with actual TTree.cxx sources. I will add couple of comments to fix small typos.

@phsft-bot
Copy link

Starting build on ROOT-performance-centos7-multicore/default, ROOT-fedora27/noimt, ROOT-fedora29/python3, ROOT-fedora30/cxx14, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu18.04-i386/cxx14, mac1014/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link

Build failed on ROOT-fedora27/noimt.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-performance-centos7-multicore/default.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-ubuntu16/nortcxxmod.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-fedora30/cxx14.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-fedora29/python3.
See console output.

Warnings:

Failing tests:

@phsft-bot
Copy link

Build failed on ROOT-ubuntu18.04-i386/cxx14.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on mac1014/cxx17.
See console output.

Warnings:

@oshadura
Copy link
Collaborator

@phsft-bot build!

@phsft-bot
Copy link

Starting build on ROOT-performance-centos7-multicore/default, ROOT-fedora27/noimt, ROOT-fedora29/python3, ROOT-fedora30/cxx14, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu18.04-i386/cxx14, mac1014/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link

Build failed on ROOT-performance-centos7-multicore/default.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-fedora29/python3.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-fedora30/cxx14.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-ubuntu16/nortcxxmod.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-fedora27/noimt.
See console output.

Warnings:

@phsft-bot
Copy link

Build failed on ROOT-ubuntu18.04-i386/cxx14.
See console output.

Warnings:

@oshadura
Copy link
Collaborator

@kamahori no worries about test failures, it is unrelated to your changes

@oshadura
Copy link
Collaborator

oshadura commented May 4, 2020

@pcanal can you please review this PR? and give some suggestions how it could be improved extended?

Copy link
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

Thanks. I guess the comment requesting additional tests can be addressed in another PR.

@guitargeek guitargeek requested a review from oshadura May 29, 2025 22:02
@guitargeek guitargeek merged commit 4f570ad into root-project:master May 29, 2025
42 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants