-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
docs(recipes/nestia): new library and new content #2571
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got a couple of changes like removing extra whitespace, and some questions I want answered, but overall it looks good! Excited to find a side project to use this on to see how it goes 😄
Co-authored-by: Jay McDoniel <[email protected]>
Co-authored-by: Jay McDoniel <[email protected]>
Co-authored-by: Jay McDoniel <[email protected]>
Co-authored-by: Jay McDoniel <[email protected]>
Co-authored-by: Jay McDoniel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed following your advices, but cannot sure how to resolve the "client code".
Would it better to remove the last example code and write like below?:
Demonstration
When you run npx nestia sdk
command, @nestia/sdk
will generate an SDK library interacting with your backend server, composed with codes like below. If you want to learn how to distribute the SDK library, visit and read Guide Documents - Distribution.
import { Fetcher, IConnection } from "@nestia/fetcher";
import { IBbsArticle } from "../../../structures/IBbsArticle";
/**
* Store a new content.
*
* @param input Content to store
* @returns Newly archived article
*/
export function store(
connection: api.IConnection,
input: IBbsArticle.IStore
): Promise<IBbsArticle> {
return Fetcher.fetch(
connection,
store.ENCRYPTED,
store.METHOD,
store.path(),
input
);
}
export namespace store {
export const METHOD = "POST" as const;
export function path(): string {
return "/bbs/articles";
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall addition looks good to me. I'll leave @kamilmysliwiec to review it as well when he has the time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Erased the last client example code.
Thanks for your guide @jmcdo29
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a break line after third party package and is not managed by the NestJS core team
~.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed guide documents (wiki) link (I'd revised contents of guide documents)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kamilmysliwiec will we bring this to the docs? looks good to me.
We already have that at https://github.com/nestjs/awesome-nestjs
Fantastic job anyway, Nestia looks really impressive! |
Well, I have waited your review for one year, and during the year, I had saw lots of other libraries (had committed later than me, even now) are newly added in the recipes section. I can understand why are you saying such like that, but it sounds collapsed for me because of long time waiting. Hope your mercy.
|
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
To introduce new library nestia
Does this PR introduce a breaking change?
Other information
Re-challenge after 6 months. During the 6 months.
Hope my libraries to be helpful for NestJS developers.