NWChemEx / .github

GitHub Settings for the NWChemEx Organization
https://nwchemex.github.io/.github/
Apache License 2.0
1 stars 2 forks source link

New ci doc #119

Closed yzhang-23 closed 10 months ago

yzhang-23 commented 1 year ago

PR Type

Brief Description Documentation on the docker-image-based CI. Currently I have not removed the docs on the old CI, since we are in the process of replacing the old CI.

Not In Scope

PR Checklist

TODOs

yzhang-23 commented 1 year ago

Finally, what you have doesn't follow our documentation conventions.

Can you be specific on this? I tried to follow the conventions. Some lines contains more than 80 characters because there are long urls (which I don't want to break into two lines) in them.

ryanmrichard commented 1 year ago

Finally, what you have doesn't follow our documentation conventions.

Can you be specific on this? I tried to follow the conventions. Some lines contains more than 80 characters because there are long urls (which I don't want to break into two lines) in them.

I'm okay with the URLs (though note this) it was your underlining/overlining that jumped out at me. Also note that Sphinx will gripe if you don't have under/overlines which match your title length.

yzhang-23 commented 1 year ago

Finally, what you have doesn't follow our documentation conventions.

Can you be specific on this? I tried to follow the conventions. Some lines contains more than 80 characters because there are long urls (which I don't want to break into two lines) in them.

I'm okay with the URLs (though note this) it was your underlining/overlining that jumped out at me. Also note that Sphinx will gripe if you don't have under/overlines which match your title length.

Underline format fixed. I changed the title but forgot to change the underline correspondingly. Thank you for pointing this out!

yzhang-23 commented 1 year ago

I can generate the webpages only if I change the setting of docutils to docutils>=0.20 on my local machine, while this setting won't work in CI. This is very annoying. It looks that sphinx has many strict version constraints.

yzhang-23 commented 1 year ago

TL;DR. IMHO what you have is really a summary of the design, not the design. I think there are plenty of things in this PR which can be reused, but we need a fully fleshed out design.

Longer review. While I could flood this PR with a multitude of comments/questions, I don't think that would be productive. Based on all of the designs I've written so far, designs are multiple PR endeavors. While this is a lot of work, I think it is worth it because: a) it easily translates into the guts of a paper, b) provides common resources for people to discuss the code, and c) it serves as a nice introduction for the next round of people who want to do development. While I'm open to getting a fully fleshed out design by other means, what follows is how I'd scope the first PR.

For background on designing for NWX see this page. If there's something not covered there, which should be covered, let's add it (one thing that comes to mind is additional resources, to which end I'll note BSSw has some). The end goal of the entire design is something similar to what I just wrote for TensorWrapper.

I'm a top-down person, meaning I'd rather start with broad generalizations and then introduce complexity as opposed to spelling out all of the complexity and then having to wrangle it into nice neat components. To that end, I'd recommend a very fundamental first PR:

* Add glossaries (I just learned about [this](https://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#glossary-directive) which makes this so much nicer).

  * I'd have one glossary for general CI terms and one for GitHub-specific terms.
  * For general terms, I'd start by defining CI, CD, and CI/CD.
  * For GitHub-specific terms, I'd start by defining actions and workflows

* Write the obligatory overview page covering:

  * Why do we want CI, CD, and/or CI/CD?
  * What high-level use cases do we want to address?
  * What are our options?
  * Why did we pick GitHub actions?
  * What is our high-level architecture/plan?

I'm thinking the overview addresses considerations which are phrased like: multiple workflows with common dependencies, needing workflows in multiple repos, different configurations of a workflow, multiple types of deployments, and having workflows with different triggers. In particular, everything in #105 should fit neatly into one of the considerations. We also want to keep the design somewhat general, by avoiding NWX-specific things. This is because when we go to write the paper we want other projects to see this information as being useful to them too.

Finally, what you have doesn't follow our documentation conventions.

Thanks for your comments. I will keep adding stuff in.

ryanmrichard commented 1 year ago

I can generate the webpages only if I change the setting of docutils to docutils>=0.20 on my local machine, while this setting won't work in CI. This is very annoying. It looks that sphinx has many strict version constraints.

Presumably this is caused by your workstation using a different version of Sphinx then? If so, we can add the needed version of Sphinx to the requirements.txt to make sure everything works.

jwaldrop107 commented 10 months ago

@yzhang-23 with the changes to the CI, I think this can be closed now.

yzhang-23 commented 10 months ago

@yzhang-23 with the changes to the CI, I think this can be closed now.

Yes. I will close it. You will have your new CI docs, right?