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

DM-42680: Add initial implementation of documentation workflow #38

Merged
merged 3 commits into from Feb 21, 2024

Conversation

JeremyMcCormick
Copy link
Collaborator

@JeremyMcCormick JeremyMcCormick commented Jan 31, 2024

Build the documentation using a compatible version of documenteer and upload for triggers other than PRs. For PRs, the documentation will be built but not uploaded. The README was the same as the documentation site, so it has been replaced by a more minimal text.

Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6701e0e) 93.36% compared to head (95ebc4b) 93.36%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #38   +/-   ##
=======================================
  Coverage   93.36%   93.36%           
=======================================
  Files          20       20           
  Lines        2109     2109           
  Branches      425      425           
=======================================
  Hits         1969     1969           
  Misses         81       81           
  Partials       59       59           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@gpdf gpdf left a comment

Choose a reason for hiding this comment

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

One small change requested

README.md Outdated
columns:
- "#sdqa_Threshold.sdqa_metricId"
mysql:engine: MyISAM
The [Felis documentation site](https://felis.lsst.io) provides more complete
Copy link

Choose a reason for hiding this comment

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

Could you move this to the top of the README, maybe as something like:

"Detailed information on usage, customization, and design is available at the Felis documentation site."

(since the sentence as it stands will read awkwardly if moved to the top unchanged)

Build the documentation using a compatible version of documenteer and
upload for triggers other than PRs. For PRs, the documentation will be
built but not uploaded.
@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-42680 branch 4 times, most recently from dbf5e9d to 240913d Compare February 21, 2024 17:43
@JeremyMcCormick
Copy link
Collaborator Author

JeremyMcCormick commented Feb 21, 2024

@gpdf Let's do a re-review of this PR. I replaced the README.md with README.rst and added some additional sections. You can view the new README here.

I don't think we should immediately link to the documentation site at the top, because it is already listed in the sidebar. The documentation as it exists is also not a good starting point for users to learn about the tool, as it starts with an overly discursive section and needs a pretty major overhaul. Maybe we could revisit this in the future once the actual site is in better shape and makes a better entry point for information? (I've still linked to it, just in a section that is closer to the bottom of the new README after the summary section. This is actually a similar README layout to several other popular tools like SQLAlchemy.)

@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-42680 branch 3 times, most recently from 74b997f to 23f6bae Compare February 21, 2024 23:39
The old README that duplicates the documentation site contents is
replaced by a shorter RST version.
Update the workflow names so that they do not have underscores in them
and are more easily readable on Github with a full description.
Copy link

@gpdf gpdf left a comment

Choose a reason for hiding this comment

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

OK to proceed

@JeremyMcCormick JeremyMcCormick merged commit 832e8e1 into main Feb 21, 2024
11 checks passed
@JeremyMcCormick JeremyMcCormick deleted the tickets/DM-42680 branch February 21, 2024 23:57
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.

None yet

2 participants