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

Update README and other documentation #711

Merged
merged 59 commits into from
Aug 14, 2023
Merged

Conversation

trey-stafford
Copy link
Contributor

@trey-stafford trey-stafford commented Aug 7, 2023

Description

Updates the README for consistency with NSIDC template (main concern being branding - added logos for NSIDC and NSF). Addresses #687.

I ended up down a bit of a documentation rabbit-hole here. As I was updating the README, a major goal was to condense the information there and place most of the responsibility for explaining e.g., contributing and usage information, on the ReadTheDocs docs.

As I started to link from the README to other parts of the documentation, I found other issues (broken links, wonky formatting, incorrect (outdated) information for QGIS 3.28, etc.) that ended up getting fixed here.

Checklist

If an item on this list is done or not needed, simply check it with [x].

  • Config lockfile updated (inv config.export > qgreenland/config/cfg-lock.json)
  • Version bumped if needed (bumpversion (major|minor|patch|prerelease|build)
  • CHANGELOG.md updated
  • Documentation updated if needed
  • New unit tests if needed

@trey-stafford
Copy link
Contributor Author

Mainly concerned with making sure we have the logos (particularly the NSIDC logo) present.

Are the other changes worthwhile? Calling out the LICENSE seems like a good idea.

The "Level of Support" section is a little awkward...it contains some contributing guidance. Should the contributing section move to under "Level of Support"?

README.md Outdated
# Level of Support

This repository is fully supported by NSIDC. If you discover any problems or
bugs, please submit an Issue. If you would like to contribute to this
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Link to issue page here? Also link to our contributing document? Should we move the contributing section here?

README.md Outdated
bugs, please submit an Issue. If you would like to contribute to this
repository, you may fork the repository and submit a pull request.

See the [LICENSE](LICENSE) for details on permissions and warranties. Please
Copy link
Contributor Author

@trey-stafford trey-stafford Aug 7, 2023

Choose a reason for hiding this comment

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

The LICENSE is called out in the "Level of Support". I thought this had been under its own section, maybe we revised the template.

Choose a reason for hiding this comment

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

I honestly don't feel we need to reference the LICENSE in the README at all, but maybe not all would agree.

The file name is all-caps, so it's practically already screaming at me, and GitHub automatically recognizes the file and displays a special link for it anyway.

If this is the way it is in the template, maybe we should have that discussion via an issue over there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you may be right. This is language taken directly from the template README.

README.md Outdated Show resolved Hide resolved
@trey-stafford trey-stafford linked an issue Aug 7, 2023 that may be closed by this pull request
README.md Outdated
contact [email protected] for more information.


# Requirements
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Requirements", "Installation", and "Usage" sections are all for QGreenland, not the code here. Is that confusing? The layout of our ReadTheDocs does a good job of distinguishing between user and contributor documentation and includes all of the information here. We should really just have very top-level information here with links to RTD.

Choose a reason for hiding this comment

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

Is that confusing?

Yes! I think we should keep the GitHub README focused on being the face of the code, and link out to RTD for user-targeted stuff.

README.md Show resolved Hide resolved
The 3.16 links give an 'outdated docs' warning. Since 3.28 is our latest
supported version, update these paths to reflect that.
Link to actual QGIS documentation page.
* Remove QGreenland plugin as indicated way that QGreenland is delivered. The
  attention admonition is sufficient.
* "Return to QGreenland website" -> "Visit the QGreenland website". This page is
  linked to from more than just he QGreenland website.
@trey-stafford trey-stafford changed the title Update README for consistency with NSIDC template. Update README and other documentation Aug 9, 2023
@trey-stafford trey-stafford marked this pull request as ready for review August 9, 2023 19:47
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated

# Getting started
This repository is fully supported by NSIDC. If you discover any problems or

Choose a reason for hiding this comment

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

I think this implies some level of USO engagement. I think we might want to set it to "unsupported" and have a broader discussion about levels of support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good point. Lets think about this some more...I'll see if I can dig up the specifics on when to use each level of service. I hate to put that this isn't supported.

Choose a reason for hiding this comment

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

Yes, it would be much better to say "community supported". I brought this up in Slack with @asteiker this week, but she's really busy with IS2 hack week. Maybe will get some feedback on that soon!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this section for now, pending further discussions at NSIDC.

README.md Outdated
bugs, please submit an Issue. If you would like to contribute to this
repository, you may fork the repository and submit a pull request.

See the [LICENSE](LICENSE) for details on permissions and warranties. Please

Choose a reason for hiding this comment

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

I honestly don't feel we need to reference the LICENSE in the README at all, but maybe not all would agree.

The file name is all-caps, so it's practically already screaming at me, and GitHub automatically recognizes the file and displays a special link for it anyway.

If this is the way it is in the template, maybe we should have that discussion via an issue over there.

README.md Outdated

## What's inside the QGreenland Core zip package?
### What is inside the QGreenland Core zip package?

Choose a reason for hiding this comment

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

You let this one slide for too long 😆

Refer to our documentation page on [Adding New Datasets to
QGreenland](/user/how-to/adding-data.md) for instructions on how to add new data
layers to your QGreenland project. Note that this section is also incldued in
the `UserGuide.pdf` included in the QGreenland Core package.

Choose a reason for hiding this comment

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

💯

doc/user/tutorials/video-series-overview.md Outdated Show resolved Hide resolved
If you are not familiar with geographic information systems (GIS) and geospatial data, this video will introduce you to GIS and what it can be used for.
### [Session 1: Introduction to QGreenland](https://www.youtube.com/watch?v=gD0vkP5JUmA&list=PLSRiyMridUCwyu-vqpAFtm8bVERgTvs7q&index=1)
If you are not familiar with geographic information systems (GIS) and geospatial
data, this video will introduce you to GIS and what it can be used for.

Choose a reason for hiding this comment

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

💯

doc/what_is_qgr.md Outdated Show resolved Hide resolved
(for example, due to filesize constraints). QGreenland Custom has its own
[documentation](https://qgreenland-plugin.readthedocs.io).
```{attention}
The QGIS Plugin **[QGreenland Custom](https://plugins.qgis.org/plugins/qgreenland/)

Choose a reason for hiding this comment

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

I like this much better!

trey-stafford and others added 13 commits August 10, 2023 08:41
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
We want to direct those who want to use QGreenland Core to our website.
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Should be obvious that qgr core is required to look at layer metadata. We do a
good job of directing users to the 'right' starting place already.

The info about the tutorial is better suited for a note.
We need some updated guidance on when/how to set the LoS. This is primarily for
tools supported by USO, which we don't really expect for QGreenland.
@trey-stafford
Copy link
Contributor Author

The conversation is 'outdated' becuase it was part of the LoS that I removed, but responding to this quote @MattF-NSIDC made:

The file name [LICENSE] is all-caps, so it's practically already screaming at me, and GitHub automatically recognizes the file and displays a special link for it anyway.

The link is the key part here. I see a "View license" link that links to our LICENSE. However, we shouldn't necessarily count on individuals seeing a file in the directory listing. When viewing a repo from a mobile browser, for example, GH hides the repo's content and just shows the README by default.


```{note}
To see the layer's geospatial metadata (e.g., the coordinate reference system
and spatial extent), we recommend [using QGIS](#via-qgis-layer-properties).

Choose a reason for hiding this comment

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

These anchor links can be broken easily if someone changes the title of the section. If we use an explicit cross-reference, these links are more durable and can throw errors at build time if we break either end of the crossref!

https://docs.readthedocs.io/en/stable/guides/cross-referencing-with-sphinx.html#explicit-targets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, I'll take a look at this! Note that our build process already produces an error if one of these links is broken (at least within single pages)

Choose a reason for hiding this comment

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

Oh cool, I thought it just skipped checks for this style of link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's just http links that get skipped. I replaced a bunch with references #like-this in other parts of this PR. In a couple of cases I changed a title without the #like-this link, and our build process thew an error indicating where those broken links were.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue to address this more holistically: #723

Choose a reason for hiding this comment

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

Nice!

@trey-stafford trey-stafford merged commit 0eb2e9d into main Aug 14, 2023
2 checks passed
@trey-stafford trey-stafford deleted the update-readme-nsidc-template branch August 14, 2023 21:31
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.

Update/review README for consistency w/ nsidc repo template
2 participants