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 #186

Merged
merged 2 commits into from
Aug 20, 2024
Merged

Update README #186

merged 2 commits into from
Aug 20, 2024

Conversation

msricher
Copy link
Contributor

@msricher msricher commented Jun 24, 2024

Updates the README to be consistent with other QC-Devs projects.

I moved the installation notes into the website and linked to it from the README.

Since there is no release yet, and CodeFactor is not integrated, I have commented out those "badge" images in the README header for now.

Checklist

  • Write a good description of what the PR does.
  • Add tests for each unit of code added (e.g. function, class)
  • Update documentation
  • Squash commits that can be grouped together
  • Rebase onto master

Type of Changes

Type
🐛 Bug fix
✨ New feature
🔨 Refactoring
📜 Docs

Related

Copy link
Member

@PaulWAyers PaulWAyers left a comment

Choose a reason for hiding this comment

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

I would say that this is a partial list of features, as the Readme may rarely be entirely up-to-date. Perhaps say, instead,

Selected Features

or

Features (Partial List)

@msricher
Copy link
Contributor Author

I would say that this is a partial list of features, as the Readme may rarely be entirely up-to-date. Perhaps say, instead,

Selected Features

or

Features (Partial List)

Done. I also linked some parts to the website.

Copy link
Member

@FarnazH FarnazH left a comment

Choose a reason for hiding this comment

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

@msricher thanks for your PR. I made some updates to the README. I wonder whether we should keep a list of features in the README. The webpage can include a concise feature list on its 1st page, but similar to iodata and grid, it would be good to delegate such information to the website. What do you think @PaulWAyers and @leila-pujal?

## Features
Following features are supported in `gbasis`:

[See the website for installation instructions.](https://gbasis.qcdevs.org/installation.html)
Copy link
Member

Choose a reason for hiding this comment

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

@msricher, before referring to the website, please include instructions for pip installation similar to grid and iodata? In the same fashion, make sure to use qc-gbasis.

Copy link
Member

Choose a reason for hiding this comment

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

I'm OK with a full list in the readme, but it's essential for the web site. libcint does a full(ish) list in the readme, so there is precedent for that. I think the readme has a different (more technical) audience than the web site.

My hesitation at making a full list in the readme is that then we have to update not just the web site but also the readme as we update the package. It's simpler to just update one of them, and in that case the readme should list selected features, not everything, and make it clear that it is just the selected web sites.

@FarnazH FarnazH requested a review from PaulWAyers August 7, 2024 16:13
@FarnazH FarnazH self-requested a review August 20, 2024 21:12
Copy link
Member

@FarnazH FarnazH left a comment

Choose a reason for hiding this comment

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

I am merging this because the current README is outdated and wrong in some instances.

@FarnazH FarnazH merged commit ef1574a into theochem:master Aug 20, 2024
0 of 10 checks passed
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.

3 participants