pharmaverse / admiralci

CI/CD workflows used across the admiral family of packages
https://pharmaverse.github.io/admiralci
Other
13 stars 4 forks source link

Documentation #175

Closed dgrassellyb closed 9 months ago

dgrassellyb commented 11 months ago

I also made this cheatsheet (However some links need the documentation to be deployed) But maybe still a bit complex to read !

Template cheatsheet.pdf

manciniedoardo commented 11 months ago

Can we add a hex to the front page too? smth like below

manciniedoardo commented 11 months ago

admiralci_logo

dgrassellyb commented 11 months ago

Thanks @dgrassellyb for this, this is fantastic!!

I left a few specific comments for typos and requests for clarification. I also have some comments about the front page README:

  • Many heading links in the readme don't work?
  • Full stop missing at the end of renv propagation section
  • Boilerplate or boilderplate? Could be a typo.
  • docker image build: typo in "each R version" (no "s" at the end)

I also have some general comments:

  • Within the navbar, for the User Guides section I would put a subsection titled "Basics" with the "Get Started", "Common structure of the workflows" and "Codespaces" sections. Then I would put another subsection titled "Invididual workflows" with all the other pages.
  • I would retitile "common structure" to "Common strucure of the workflows" and put it at the top of the articles.
  • There's also another "spell checks page" in the Articles drop down. Not sure what the difference is between the two?

Thanks a lot for your feedbacks, I will review this ! :)

bms63 commented 11 months ago

I also made this cheatsheet (However some links need the documentation to be deployed) But maybe still a bit complex to read !

Template cheatsheet.pdf

above and beyond!!! This cheatsheet is so cool

bms63 commented 11 months ago

Just a few notes:

Musing on the README - should we just remove most of the stuff there as it is discussed more thoroughly in the vignettes.

Get Started: Could this vignette get a separate tab?

Cheatsheet: Could that be linked as a Get Started dropdown option? Like how we have it on https://pharmaverse.github.io/admiral/dev/

Lock Files and Propagation: Since we are moving away from this - wondering if this is still need to document?

dgrassellyb commented 9 months ago

@bms63 / @manciniedoardo / @cicdguy / @bundfussr : I am currently updating the doc (quite lot of changes since we removed renv !) - I just have a question about codespaces, our actual config is quite outdated (since we built it from renv dependencies) - I could work on this, but while it's not ready I was just thinking to tag the codespace section as "WIP article", what do you think ?

I guess codespaces feature is a bit low priority right ? (I am not sure if devs are really using it on admiral but let me know if I am wrong ! πŸ™Š )

cicdguy commented 9 months ago

I'd vote to keep at as a "WIP article" and get back to it later.

dgrassellyb commented 9 months ago

@bms63 / @manciniedoardo / @cicdguy / @bundfussr : I am currently updating the doc (quite lot of changes since we removed renv !) - I just have a question about codespaces, our actual config is quite outdated (since we built it from renv dependencies) - I could work on this, but while it's not ready I was just thinking to tag the codespace section as "WIP article", what do you think ?

I guess codespaces feature is a bit low priority right ? (I am not sure if devs are really using it on admiral but let me know if I am wrong ! πŸ™Š )

I'd vote to keep at as a "WIP article" and get back to it later.

Ok let's do this ! it's ready to be merge now (I also pushed a fix in pkgdown to be able to push non-multi version documentation - because I guess we don't need multi version for admiralci doc !). I also updated the cheatsheet here : https://github.com/pharmaverse/admiralci/blob/documentation/vignettes/assets/cheatsheet.png

bms63 commented 9 months ago

Shall we just merge and publish this and do the review in an issue? Might be a little easier if we don't have to pull down and render the site??

dgrassellyb commented 9 months ago

Shall we just merge and publish this and do the review in an issue? Might be a little easier if we don't have to pull down and render the site??

I'll fork it for rendering and send you the link

dgrassellyb commented 9 months ago

Pushed again some other fixs, here is the rendered doc from my forked repo : https://dgrassellyb.github.io/admiralci/ However .. I have an issue in r cmd checks for this PR - not sure to understand where it's coming from :

❯ checking CRAN incoming feasibility ... [1s/13s] NOTE
  Maintainer: β€˜Open Source <admiralci@pharmaverse.com>’

  Found the following (possibly) invalid URLs:
    URL: 
      From: inst/doc/push-docker.html
      Message: Empty URL

0 errors βœ” | 0 warnings βœ” | 1 note βœ–

I even updated the .Buildignore with ^inst/doc\.html$ but still get this

cicdguy commented 9 months ago

@dgrassellyb That regex looks incorrect. Should be ^inst\/doc\/.*.html$

bms63 commented 9 months ago

Can we minimize this image please image

bms63 commented 9 months ago

Can we change the tab name to CI/CD Workflows image

When I get a dozen tabs up be nice if I new which admiral to select??

bms63 commented 9 months ago

What about removing these links and just listing out our potential workflows? The one I clicked go to 404.

image

Can we say why this is a bad practice? can't get latest and greatest updates - encourage you to make issues to enchance our workflows.

image

Could we give some context on what these bullet points are leading us to?

image

bms63 commented 9 months ago

Possible to embed cheat sheet like we did for admiral? image

Shall we remove devel references image

Many thanks again!! Could be a really good reference for discussion for upcoming admiralpeds @rossfarrugia

dgrassellyb commented 9 months ago

Possible to embed cheat sheet like we did for admiral? image

Shall we remove devel references image

Many thanks again!! Could be a really good reference for discussion for upcoming admiralpeds @rossfarrugia

Thanks Ben I will apply some changes as you suggested (and also for your other comments above ;) )

dgrassellyb commented 9 months ago

What about removing these links and just listing out our potential workflows? The one I clicked go to 404.

image

Can we say why this is a bad practice? can't get latest and greatest updates - encourage you to make issues to enchance our workflows.

image

Could we give some context on what these bullet points are leading us to?

image

For the links it's not working because it's coming from my fork (if you replace on links pharmaverse by dgrassellyb it will work) - we won't have issues with links once it will be deployed in admiralci repo (non-forked one !) - I think it's nice to keep it (it's like a table of contents with redirections to detail documentation + workflows code)