-
-
Notifications
You must be signed in to change notification settings - Fork 193
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
fix: issue with not reading spec file in working directory when no global context is created #48
Conversation
…obal context is set.
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.
Works like a charm mate, but we need to put more power into developer experience
src/messages.tsx
Outdated
@@ -14,3 +14,5 @@ export const ValidationMessage = (filePath: string) => ({ | |||
error: () => `File: ${filePath} does not exists or is not a file!`, | |||
message: () => `File: ${filePath} successfully validated!` | |||
}); | |||
|
|||
export const NO_SPEC_PATH_FOUND = 'No spec path found in your working directory, please use flags or store a context'; |
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.
We need to do better here. User, when running asyncapi validate
most probably do not have an idea about all our options, defaults, and way we load files, priorities - we need to make it clear. Good help messages are a key for good developer experience.
how about? 👇🏼
export const NO_SPEC_PATH_FOUND = 'No spec path found in your working directory, please use flags or store a context'; | |
export const NO_SPEC_FOUND = 'Unable to perform validation. Specify what AsyncAPI file should be validated.\n\nThese are your options to specify in the CLI what AsyncAPI file should be used:\n- You can use --file flag to provide a path to the AsyncAPI file: asyncapi validate --file path/to/file/asyncapi.yml\n- You can use --context flag to provide a name of the context that points to your AsyncAPI file: asyncapi validate --context mycontext\n- In case you did not specify a context that you want to use, the CLI checks if there is a default context and uses it. To set default context run: asyncapi context use mycontext\n- In case you did not provide any reference to AsyncAPI file and there is no default context, the CLI detects if in your current working directory you have files like asyncapi.json, asyncapi.yaml, asyncapi.yml. Just rename your file accordingly.'; |
Now, above is just a suggestion of how message should be printed, but from code perspective it should be splitted
Part Unable to perform validation. Specify what AsyncAPI file should be validated.
should be only in context of asyncapi validation
and the rest of the message should be reusable. Why reusable? because it will be always used in other commands when the same error will happen that there is no file specified, when we will have command asyncapi optimize
it will be the same. So you need to make sure that examples that I provided in the message are customizable, in context of validation user see asyncapi validate --file path/to/file/asyncapi.yml
, but in context of optimization user will see asyncapi optimize --file path/to/file/asyncapi.yml
@Souvikns please have a look at my comments again, I suggested that part of the message must be reusable and things like |
Looking at the message I would see only the name of the command in the example is dynamic. Suppose I am using the So basically we could pass a command parameter to the import meow from 'meow';
import { injectable } from 'tsyringe';
injectable();
export class CLIService {
private _cli = meow('', { argv: process.argv.slice(2), autoHelp: false })
command(): string {
return this._cli.input[0] || '';
}
args(): any {
return this._cli.flags;
}
} |
I won't be able to reuse it with |
maybe we can make an object which stores all these fallback messages and after we determine which command was used print them accordingly. |
Can't we just split |
If we divide the complete message then
We can split them and then connect them in the error to use accordingly. |
exactly, split and then reuse together or in different configurations |
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.
Great work!
🎉 This PR is included in version 0.3.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…obal context is set.
Description
Related issue(s)
Fixes #35