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

Aws docs n8 #167

Merged
merged 6 commits into from Jul 10, 2020
Merged

Aws docs n8 #167

merged 6 commits into from Jul 10, 2020

Conversation

vtnate
Copy link
Contributor

@vtnate vtnate commented Jul 6, 2020

Addresses #167

Pull Request Description

  • Make docs a little clearer
  • Add link to bsb github repo
  • Make AWS CLI version 2 the obviously correct choice. Eliminate outdated advice about venvs, not necessary with version 2
  • Remove PII from example yml file
  • Include note that dashes are not allowed in project_identifier for AWS

Checklist

Not all may apply

  • Code changes (must work)
  • Tests exercising your feature/bug fix (check coverage report on CircleCI build -> Artifacts)
  • All other unit tests passing
  • Update validation for project config yaml file changes
  • Update existing documentation
  • Run a small batch run to make sure it all works (local is fine, unless an Eagle specific feature)
  • Add to the changelog_dev.rst file and propose migration text in the pull request

@vtnate vtnate self-assigned this Jul 6, 2020
@vtnate vtnate requested a review from nmerket July 6, 2020 21:59
@vtnate vtnate mentioned this pull request Jul 6, 2020
@vtnate
Copy link
Contributor Author

vtnate commented Jul 6, 2020

@nmerket This should make things a little easier for AWS users.

Did I update the changelog file the way you want? :)

Copy link
Member

@nmerket nmerket left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

docs/installation.rst Show resolved Hide resolved
docs/installation.rst Outdated Show resolved Hide resolved
docs/installation.rst Show resolved Hide resolved
Comment on lines +18 to +23
.. change::
:tags: documentation
:pullreq: 167
:tickets: 168

Minor updates to documentation.
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 on the fence about whether we need to document in the changelog minor things like this. What do you think, @rHorsey?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like this is not worthy of changelog, as nothing in the code is actually changing.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vtnate @nmerket As written def not impactful, but I actually think tracking docs changes is critical, not least of all because we seriously need to start 'ataboy-ing' every docs improvement. Also if I go read something and I'm like - since when has this been the recommended way of doing whatever - I can still go find the change to the docs and then hopefully reverse-engineer what's up. I'd just slightly change this to read Made minor clarity updates in configuring local docker and aws setups.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I really like the idea of every PR that get's merged having an entry - it's something that I see in projects like dask and sqlalchemy and I really like what it does to the culture of development and use of the projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dunno if it's appropriate here, but in urbanopt we're following the openstudio-extension changelog pattern, and using that script to access the github issues for direct copy/paste into the changelog. It's pretty slick, in my experience, at letting you run the script just before a release and updating the changelog easily. Here is an example changelog built using that script.

Co-authored-by: Noel Merket <noel.merket@nrel.gov>
@nmerket nmerket merged commit 2bb8dcd into master Jul 10, 2020
@nmerket nmerket deleted the aws-docs-n8 branch July 10, 2020 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants