Skip to content

Conversation

@orshlom
Copy link

@orshlom orshlom commented Feb 4, 2019

Initial documentation for the new "lb4 relation" CLI command.

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@bajtos
Copy link
Member

bajtos commented Feb 4, 2019

@orshlom thank you for the pull request. I love the idea of starting with the documentation describing the user experience we want to build 👍

On the other hand, landing the documentation only is not very useful, as we can eventually get to to the state where the docs are ahead of the implementation. Ideally, documentation should be a part of the pull request implementing the actual feature.

I am proposing to change the title of this pull request to make it clear that it will eventually contain both the docs and the implementation, and once you get enough feedback on the docs, you can start adding the implementation to this very same pull request.

Thoughts?

@bajtos bajtos added feature CLI community contribution Relations Model relations (has many, etc.) labels Feb 4, 2019
@orshlom
Copy link
Author

orshlom commented Feb 4, 2019

Hi @bajtos,
The implementation is in progress on another pull request #2223.
Because they are from different types, we were thinking about creating two pull requests (one for docs and one for the new feature).
Do you want us to add the documentation over there?

@bajtos
Copy link
Member

bajtos commented Feb 5, 2019

Because they are from different types, we were thinking about creating two pull requests (one for docs and one for the new feature).
Do you want us to add the documentation over there?

Yes please, keep the documentation together with the implementation in the same pull request. It makes it easier to build understanding of the proposed implementation, and then add documentation for any important edge cases discovered during review.

BTW that's one of the reasons why our pull request check-list has an item "Documentation in /docs/site was updated.

@orshlom orshlom reopened this Feb 5, 2019
@orshlom orshlom closed this Feb 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLI feature Relations Model relations (has many, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants