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

Clean up "Best Practices with LoopBack 4" #1094

Closed
11 tasks done
shimks opened this issue Mar 6, 2018 · 10 comments · Fixed by #1433
Closed
11 tasks done

Clean up "Best Practices with LoopBack 4" #1094

shimks opened this issue Mar 6, 2018 · 10 comments · Fixed by #1433
Assignees

Comments

@shimks
Copy link
Contributor

shimks commented Mar 6, 2018

I've been reviewing docs for Best Practices with LoopBack 4 as a part of #988 and had a couple of concerns about the whole section in context to the current timeline of the framework.

I have a problem with this part of the docs in general. Currently, the section goes in depth on building an application using the top-down approach, which is not supported atm. Since the section itself is more or less a tutorial on an outdated codebase and preaches itself as the 'best practice', I personally think whole section should be renamed and shelved until we come back to supporting top-down approach. I've arrived at this decision after wanting to use the latest openapi-v3 package for our best practices, and noticing that it doesn't fully support top-down approach.

Here's what I'd like to propose:

  • rename Best-practices-with-Loopback-4.md to something else; as long as it doesn't have the words 'best practices' in it, I'm happy
  • create issues that will plan out the replacement of the content long-term, and potentially short-term

Acceptance Criteria

  • Rename Best practices with Loopback 4 to be Best Practices in Docs & Sidebar - see docs: rename Best Practices page & sidebar section #1417
    • Update the main Best Practices page to reflect the new content under this section (see below)
    • Move Define the API using code-first approach under this section
    • Move Defining your Testing Strategy under this section
    • Move Testing your Application under this section (see doc: list Testing guides under Best Practices #1276)
  • Remove Defining the API using design-first approach from sidebar
    • Remove Testing the api under this section from sidebar as well
  • Remove implementing Features from sidebar
  • Remove Preparing the API for consumption from sidebar
  • For pages removed from the sidebar, rename the file to have a .shelved.md suffix so we know in future the page was shelved and it's not having it in the sidebar is not an oversight.
  • Ensure no docs reference the removed pages / update the pages accordingly to remove the links if they have a reference (Check all Docs + Readme's)
@b-admike
Copy link
Contributor

b-admike commented Mar 7, 2018

+1 to renaming the section. I also agree that the tutorial should be revamped.

@jannyHou
Copy link
Contributor

jannyHou commented Mar 7, 2018

Good point @shimks 👍

I think "building API with code" and "building API with existing OpenAPI spec" (short for top-down) are two approaches based on different user scenarios, but the approaches themselves are on the same level, they don't take a precedence on each other.

Given the name "best practice", we can treat the "defining APIs by resources" as the best practice for top-down approach, especially when you have existing OpenAPI spec. But it would be better to clarify, for users who build an app from scratch, it's on their choice to take the top-down approach or not.

Another concern from me is our monorepo doesn't provide many help to simplify organizing/joining spec fragments, so

  • I would expect some high level prospect on this: is it still the direction we are going to take?
  • If yes, I think we can enhance @loopback/openapi-spec-builder to provide more functions to build/integrate spec fragments.

@virkt25
Copy link
Contributor

virkt25 commented Mar 9, 2018

+1 to just shelving this page in it's entirety. The page is a challenge to maintain and takes up resources that should be used to reach MVP. Keeping this page up-to-date with frequent changes to the code base will continue taking up resources.

I would agree that the page be renamed (Design First Approach) and a warning added. If the team agrees, I would suggest just dropping the page entirely from the side bar and we can re-visit the page to re-write or update it once we have a final stable release.

@bajtos
Copy link
Member

bajtos commented Mar 16, 2018

My thoughts as the original author of "Best Practices with LoopBack 4": I agree that some parts are specific to design-first approach, I don't mind moving them out of sight. However, there are parts that are still highly relevant:

  • Defining your testing strategy contains generic advice applicable to both design-first and code-first styles. My proposal: keep this page visible.
  • The code examples in Implementing features assume design-first style, but a lot of the content (and advice) is applicable to code-first style. My proposal: rework this page to accommodate for both design-first and code-first styles.

Please note that Testing your application is referring to some of these best practices too, extra care must be taken to avoid broken links.

@shimks
Copy link
Contributor Author

shimks commented Mar 16, 2018

@bajtos I agree that those pages, or at least their contents, should be kept.

  • For Defining your testing strategy, I think its theme overlaps with Testing your application and I'm honestly not sure what to do about these two files. Testing your application gives examples of various levels of testing (unit, integration, etc.) along with some good practices, but it's not really describing the TDD process described in Defining your testing strategy. I agree that Defining your testing strategy should live under what we preach as 'best practices', but I guess I'm confused about Testing your application's place in our docs.
    I kind of want to consolidate the two together, but I'm curious as to what you think of as the author of both of those two pages.
  • As for Implementing features, I think we can follow what we discussed in this PR (Add best practices for bottom up approach loopback.io#624 (comment)) to rework the content.

@shimks
Copy link
Contributor Author

shimks commented Mar 16, 2018

Additionally, I think this issue be broken down into two separate PRs:

  • PR/issue to shelve Best-practices-with-LoopBack with the exception of Defining-your-testing-strategy
    • Rationale: the code and the approach from this section is outdated, with the exception of testing-strategy, so we should take it off of loopback.io. This can also be done quickly to fit in with the scope of Review existing documentation #988
  • PR/issue to re-introduce Best-practices-with-LoopBack with reworked content

@virkt25
Copy link
Contributor

virkt25 commented Mar 20, 2018

For pages that end up being shelved, we just need to remove them from the sidebar IMO and don't actually need to rename the page / file. If someone has a direct link, they can read the page still.

Once the page is re-worked, it can be added to the sidebar again.

@bajtos
Copy link
Member

bajtos commented Mar 22, 2018

I am proposing to add "Defining your testing strategy" as a top-level entry under "Best practices" section in the left sidebar, and then remove the other pages from the sidebar as proposed by @virkt25. I think "Testing your application" should be nested under "Best practices" in the sidebar too.

@bajtos
Copy link
Member

bajtos commented Mar 22, 2018

For Defining your testing strategy, I think its theme overlaps with Testing your application and I'm honestly not sure what to do about these two files. Testing your application gives examples of various levels of testing (unit, integration, etc.) along with some good practices, but it's not really describing the TDD process described in Defining your testing strategy. I agree that Defining your testing strategy should live under what we preach as 'best practices', but I guess I'm confused about Testing your application's place in our docs.
I kind of want to consolidate the two together, but I'm curious as to what you think of as the author of both of those two pages.

In my view, these two pages have different goals and should be preserved both.

  • "Defining your testing strategy" explains how to approach testing at conceptual/strategic level.
  • "Testing your application" is a practical guide showing how to write and structure your test code.

@virkt25
Copy link
Contributor

virkt25 commented Mar 22, 2018

+1 to @bajtos proposal for a new "Best Practices" section that houses "Defining your testing strategy" and "Testing your application" while other pages are removed from the side bar till LB4 is more stable and those pages can be updated.

@bajtos bajtos changed the title Best Practices with LoopBack 4 should be renamed Clean up Best Practices with LoopBack 4 May 29, 2018
@bajtos bajtos changed the title Clean up Best Practices with LoopBack 4 Clean up "Best Practices with LoopBack 4" May 29, 2018
@bajtos bajtos self-assigned this Jun 12, 2018
@bajtos bajtos added the p1 label Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants