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

Internal dev README (potentially user-facing README) #205

Merged
merged 8 commits into from
Oct 6, 2022

Conversation

dbogunowicz
Copy link
Contributor

A simple guide to using the new sparsezoo.

I would like this document to end up as a part of our user-facing docs. Let's discuss what properties this would need to have to be as useful as possible for our users.

@dbogunowicz dbogunowicz requested review from bfineran, anmarques, a team and corey-nm and removed request for a team August 4, 2022 10:51
@dbogunowicz dbogunowicz changed the title Internal dev README (potential user-facing README) Internal dev README (potentially user-facing README) Aug 4, 2022
corey-nm
corey-nm previously approved these changes Aug 8, 2022
Copy link
Contributor

@corey-nm corey-nm left a comment

Choose a reason for hiding this comment

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

Nice document!

README_.md Outdated Show resolved Hide resolved
@corey-nm
Copy link
Contributor

@dbogunowicz what's the plan with this doc? still want to merge or no?

@dbogunowicz
Copy link
Contributor Author

@corey-nm absolutely. It ended up in limbo, waiting for reviews. Let me ping the rest of the team to decide how to complete this PR.

bfineran
bfineran previously approved these changes Sep 27, 2022
Copy link
Contributor

@bfineran bfineran left a comment

Choose a reason for hiding this comment

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

LGTM overall. Please re verify all code examples before landing since it's been a few months

README_.md Outdated Show resolved Hide resolved
KSGulin
KSGulin previously approved these changes Sep 27, 2022
@dbogunowicz dbogunowicz dismissed stale reviews from KSGulin, bfineran, and corey-nm via 9985434 September 27, 2022 17:05
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@dbogunowicz dbogunowicz requested review from jeanniefinks and removed request for anmarques September 27, 2022 18:52
@dbogunowicz
Copy link
Contributor Author

Revisited the PR, fused my write-up with the repositories README.md, added some missing features to the documentation, and tested all the pathways.

corey-nm
corey-nm previously approved these changes Sep 27, 2022
Copy link
Contributor

@corey-nm corey-nm left a comment

Choose a reason for hiding this comment

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

🎉

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@jeanniefinks jeanniefinks left a comment

Choose a reason for hiding this comment

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

Reads great! Minor minor comments!

Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>
README.md Outdated Show resolved Hide resolved
Copy link
Member

@markurtz markurtz left a comment

Choose a reason for hiding this comment

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

@dbogunowicz can we get this cherry picked over to 1.2 as well?

@dbogunowicz
Copy link
Contributor Author

@markurtz for sure

@dbogunowicz dbogunowicz dismissed jeanniefinks’s stale review October 6, 2022 08:17

Want to merge and cherry pick it before release. I took all Jeannie's comments into consideration.

@dbogunowicz dbogunowicz merged commit 88b330d into main Oct 6, 2022
@dbogunowicz dbogunowicz deleted the docs/internal_readme branch October 6, 2022 08:17
dbogunowicz added a commit that referenced this pull request Oct 6, 2022
* initial commit

* merge readmes

* just need grammar and consistency review

* Apply suggestions from code review

* Update README.md

Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>

* Update README.md

Co-authored-by: bogunowicz@arrival.com <bogunowicz@arrival.com>
Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>
dbogunowicz added a commit that referenced this pull request Oct 6, 2022
* initial commit

* merge readmes

* just need grammar and consistency review

* Apply suggestions from code review

* Update README.md

Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>

* Update README.md

Co-authored-by: bogunowicz@arrival.com <bogunowicz@arrival.com>
Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>
dbogunowicz added a commit that referenced this pull request Oct 6, 2022
* initial commit

* merge readmes

* just need grammar and consistency review

* Apply suggestions from code review

* Update README.md

Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>

* Update README.md

Co-authored-by: bogunowicz@arrival.com <bogunowicz@arrival.com>
Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>
dbogunowicz added a commit that referenced this pull request Oct 6, 2022
)

* Internal dev README (potentially user-facing README) (#205)

* initial commit

* merge readmes

* just need grammar and consistency review

* Apply suggestions from code review

* Update README.md

Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>

* Update README.md

Co-authored-by: bogunowicz@arrival.com <bogunowicz@arrival.com>
Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>

* Internal dev README (potentially user-facing README) (#205)

* initial commit

* merge readmes

* just need grammar and consistency review

* Apply suggestions from code review

* Update README.md

Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>

* Update README.md

Co-authored-by: bogunowicz@arrival.com <bogunowicz@arrival.com>
Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>

Co-authored-by: bogunowicz@arrival.com <bogunowicz@arrival.com>
Co-authored-by: Jeannie Finks <74554921+jeanniefinks@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants