-
Notifications
You must be signed in to change notification settings - Fork 12
Added details of oneDNN CI coverage and needs #218
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
Conversation
c33bc61
to
75cd56a
Compare
| Owner | Target | OS | Concurrency | Active? | How to access logs | | ||
| -------------- | ----------- | --------------------- | ------------ | ------- | ------------------- | | ||
| @vpirogov | CPU x64 | Linux, Windows, macOS | 2 | Yes | CI x64 PR check | | ||
| @theComputeKid | CPU AArch64 | Linux, macOS | 2 | Yes | CI AArch64 PR check | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a public Nightly AArch64 as well, where we do more extensive testing and also basic performance sanity checks. Do we want to include that? It is different to the PR checks, but can be triggered on a branch. This would be outside the definition of CI as you described in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the results public and how is it triggered? If it has public results I think we should document it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is public and triggered both manually and automatically (every night).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the results public and how is it triggered? If it has public results I think we should document it here.
This testing runs on main branch and does not help with making decisions on accepting PRs, which was the original definition. Do we want to clarify what 'CI' means in this document so that there's no confusion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point. Perhaps we need a section for regular build/test that happens alongside other things. We could potentially harness this for CI in the future in any case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable. @theComputeKid, do you want to take a stab at that with the next set of changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'll merge this information now and they can add this information later.
Could you change lowercase |
You could make a new PR with the change |
I'll include this change. |
aa7620f
to
a4e2d64
Compare
a4e2d64
to
c972001
Compare
Working assumption is that "CI" in context of this document refers to tests that trigger automatically or can be triggered manually on a PR and have publicly available results.
Closes #206.