Skip to content

Conversation

@JordanMartinez
Copy link
Contributor

@JordanMartinez JordanMartinez commented Apr 13, 2019

This is my version of #65.

People who are more inclined to study this can read through the more 
detailed section and the referenced blog posts.
It will be added at the end of the file
I don't want people to take that point out of context and then yell at 
core contributors without having read through the rest of the document
@chexxor
Copy link
Owner

chexxor commented Apr 13, 2019

Yeah this looks really good. 👍
Short and easy to read, and the solutions/outcomes aren't so clearly in there. 👌

@chexxor
Copy link
Owner

chexxor commented Apr 13, 2019

Fo you intend to direct this into master branch? Or into the big PR?

@JordanMartinez
Copy link
Contributor Author

Oh crap! It was supposed to be merged into the context-narrative branch 🤦‍♂️

@JordanMartinez JordanMartinez changed the base branch from master to context-narrative April 13, 2019 23:34
@JordanMartinez JordanMartinez marked this pull request as ready for review April 13, 2019 23:34
@JordanMartinez
Copy link
Contributor Author

Ok. Fixed and now ready for review.

| Example Type | Comment | Explanation
| - | - | - |
| Bad | We should send documentation PRs to specific libraries | Answers 'how to do something' not 'the reality that will be true when we are finished.'
| Good | **Goal:** The top 30 dependencies used in PS' ecosystem have examples and counterexamples in all of their Pursuit docs.<br>**How:** We make a version of Justin Woo's "ACME" Spago project ([project](https://github.com/justinwoo/acme-spago) & [resulting docs](https://jusrin.dev/acme-spago/)). For packages that are lacking docs or are maintained by core contributors who won't respond quickly, we could use Spago to override those packages with a version that includes more documentation. Then, via `spago docs`, would could create a local copy that has the updated docs. | States a goal but does not determine how that could be achieved. But a few ideas quicly come to mind for how.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While doing this PR, I think there should be a different example here. I fear people will click on the link to the ACME project and it will short-circuit their thought process, which (after reading this doc) is probably in its best mindset to contribute towards the doc improvement purposes.

However, I'm not sure what that example should be.

@JordanMartinez
Copy link
Contributor Author

@chexxor Anything else to review? Or should I merge this?

@chexxor
Copy link
Owner

chexxor commented Apr 16, 2019

I looked it over yesterday and I think it looks good. I think we can merge it, yeah.

@JordanMartinez
Copy link
Contributor Author

Great!

@JordanMartinez JordanMartinez merged commit 6bafbe6 into context-narrative Apr 16, 2019
@JordanMartinez JordanMartinez deleted the updateToNewStructure branch April 16, 2019 02:55
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