Skip to content
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

Initial pages site content #130

Merged
merged 1 commit into from
Sep 30, 2021
Merged

Initial pages site content #130

merged 1 commit into from
Sep 30, 2021

Conversation

trosenqu
Copy link
Contributor

Description

This is the initial gh-pages commit for phase 1 libraries open source alignment.

@trosenqu
Copy link
Contributor Author

@mmeterel @mkrainiuk @JOCwriter

I've submitted this pull request on John's behalf. He had some history conflicts and we were unsure how to resolve them without putting his current pull request at risk.

Please review and approve at your earliest convenience. This is all new content for gh-pages and we hope with this inclusion that your pages site should go live.

@trosenqu
Copy link
Contributor Author

@mmeterel nudge.
Do you think you could approve today?

@mmeterel
Copy link
Contributor

mmeterel commented Sep 28, 2021

@trosenqu @JOCwriter
Could you please provide more context on the purpose of this PR? If there was a meeting ahead of time and such PR is decided, I was not involved and I don't have any context to approve/review this PR. Some of the questions I have are:

  1. Where are these new files coming from? From the docs folder, or from the oneAPI spec documentation?
  2. Why are they placed under _source folder?
  3. Did you test if the docs are build fine with these changes?
  4. Why are we naming the files from .rst to .rst.txt?
  5. Is this PR replacing the PR opened by @JOCwriter before (Updated BLAS domain with exact copy-over from oneMKL spec per SME dir… #128) ? If so, can you please close the other one?
  6. There are several binary and .js files added with this PR. How are they used? Are they really necessary?
  7. Say this PR is merged, what is going to happen to doc folder?

I don't feel comfortable approving this PR without getting a better understanding.

@mmeterel mmeterel self-requested a review September 28, 2021 16:37
@trosenqu
Copy link
Contributor Author

trosenqu commented Sep 28, 2021

  1. These files are html files for the github pages site. It is the way you publish on github. Check in HTML into an orphan gh-pages branch. This is HTML content built from on the current rst sources on the oneMKL interfaces repo. (not the spec)
  2. The rst.txt that you see under _source is an artiface of all sphinx builds and just allows people to see the rst source if they want by clicking a button on the HTML site. I can demo if you like.
  3. These are the built docs. The were built without error
  4. See item 2 above
  5. This is not replacing the PR from @JOCwriter
  6. They are a standard part of sphinx output. Some of them support search. What are your concerns? These are already used on dozens of doc deployments in oneAPI if that helps your concern.
  7. nothing.

@mmeterel
Copy link
Contributor

mmeterel commented Sep 28, 2021

  1. These files are html files for the github pages site. It is the way you publish on github. Check in HTML into an orphan gh-pages branch. This is HTML content built from on the current rst sources on the oneMKL interfaces repo. (not the spec)
  2. The rst.txt that you see under _source is an artiface of all sphinx builds and just allows people to see the rst source if they want by clicking a button on the HTML site. I can demo if you like.
  3. These are the built docs. The were built without error
  4. See Adding cuBLAS backend to oneMKL. #2
  5. This is not replacing the PR from @JOCwriter
  6. They are a standard part of sphinx output. Some of them support search. What are your concerns? These are already used on dozens of doc deployments in oneAPI if that helps your concern.
  7. nothing.
  1. Thanks for the answers. So, what I understand is, these are the output files we get after we build the docs folder, is that right? If yes, is this the right practice to do? I would assume, we should give directions to users on how to build the docs folder and they generate these files in their system. A follow up question is, say we change the docs folder, then do we need to re-generate these files as well? Does not seem very productive.
  2. Not sure how to use the link you mentioned in question 4

@trosenqu
Copy link
Contributor Author

  1. Yes, these are the output files we get from building the docs folder in this repo. By posting to an orphan branch gh-pages the output files will automatically be served on the web for all to see that output. This is that standard Github practice that almost all the other oneAPI repos are already using (one example: https://oneapi-src.github.io/oneDAL/). We can certainly provide instructions for users to build for themselves, but I'm not sure if this is high-value. Why generate the docs on your system, when they're already available attached to this repo? Might be interesting to some. Yes, everytime you change the docs folder you'll need to re-build and re-update the gh-pages sources. I absolutely suggest that you set up CI to do this, because this too is standard practice.
  2. I updated the comment. I was referring to the second item of that same list, but Github though I meant something else and inserted that link.

@JOCwriter
Copy link
Contributor

@mmeterel , can we count on your approval by EOD today? @trosenqu has provided answers and context to your questions and we are trying to have this ready and live by launch (tomorrow). Please approve at your earliest convenience.

Regards,
John

@mmeterel
Copy link
Contributor

  1. Yes, these are the output files we get from building the docs folder in this repo. By posting to an orphan branch gh-pages the output files will automatically be served on the web for all to see that output. This is that standard Github practice that almost all the other oneAPI repos are already using (one example: https://oneapi-src.github.io/oneDAL/). We can certainly provide instructions for users to build for themselves, but I'm not sure if this is high-value. Why generate the docs on your system, when they're already available attached to this repo? Might be interesting to some. Yes, everytime you change the docs folder you'll need to re-build and re-update the gh-pages sources. I absolutely suggest that you set up CI to do this, because this too is standard practice.
  2. I updated the comment. I was referring to the second item of that same list, but Github though I meant something else and inserted that link.

IMHO, this is against the philosophy of having an open source project and encouraging developers to make contributions. The developers should be able to add new content and appropriate documentation for them, test them on their side before submitting a PR. We should not create unnecessary dependencies for the developers. This is not convenient even for within Intel developers. For example, when I change documentation, I will have to update these output files and add them to the PR as well. Ideally, we should only change the source files.
Also, we definitely need instructions for the users to generate these outputs.

@mmeterel
Copy link
Contributor

@mmeterel , can we count on your approval by EOD today? @trosenqu has provided answers and context to your questions and we are trying to have this ready and live by launch (tomorrow). Please approve at your earliest convenience.

Regards, John

@JOCwriter @trosenqu
Personally, I am not in favor of this PR due to the reasons I mentioned above.
I am adding Julia.
@jasukhar , can you please give advice on the direction here?

@trosenqu
Copy link
Contributor Author

@jasukhar @mmeterel @JOCwriter

FWIW this strategy has been used on oneTBB, oneDAL, oneCCL, oneDPL, oneVPL. and others. oneMKL is the last product not publishing documentation to Github pages in this way. Perhaps they and Robert Cohn can better answer how this approach is not against the philosophy of having an open source project.

I'm sure someone on some of these teams could help you with CI as well so that any merge to docs would trigger a re-build and update of gh-pages.

@mmeterel
Copy link
Contributor

@jasukhar @mmeterel @JOCwriter

FWIW this strategy has been used on oneTBB, oneDAL, oneCCL, oneDPL, oneVPL. and others. oneMKL is the last product not publishing documentation to Github pages in this way. Perhaps they and Robert Cohn can better answer how this approach is not against the philosophy of having an open source project.

I'm sure someone on some of these teams could help you with CI as well so that any merge to docs would trigger a re-build and update of gh-pages.

I just checked oneTBB for example (https://github.com/oneapi-src/oneTBB), and I don't see _sources directory. Can you point me to the right place to look?

This project/repo is NOT oneMKL. It is oneMKL interfaces, and the documentation here is not same as oneMKL documentation. I am against using this repos documentation for oneMKL since the beginning.

@trosenqu
Copy link
Contributor Author

Here is where oneTBB documentation is hosted: https://oneapi-src.github.io/oneTBB/

Here is the _sources directory in their repository: https://github.com/oneapi-src/oneTBB/tree/gh-pages/_sources

The build we've checked in here is built from the doc directory in this very repo. Sorry, If I gave you the wrong impression above (I don't always use the right terminology because I'm not on this project). The doc just covers BLAS, not all of oneMKL.

@trosenqu
Copy link
Contributor Author

image

This is a screen shot of opening page of what we're committing when hosted

@mmeterel
Copy link
Contributor

After chatting with Todd offline, I have a better understanding on the PR. (Thanks @trosenqu ) The agreement is, when/if there are any changes in the docs folder, documentation team will regenerate these gh-pages until an automated way is implemented. AFAIK, we currently don't support such automated/CI in this repo.

Copy link
Contributor

@mmeterel mmeterel left a comment

Choose a reason for hiding this comment

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

Pls see my comments in the conversation.

@mkrainiuk mkrainiuk merged commit 2c70511 into uxlfoundation:gh-pages Sep 30, 2021
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.

4 participants