-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
feat: add initial structure of the project and first feature to validate asyncapi files #5
Conversation
Looks good so far, I would not worry much about project structure as this can always be improved later, most important is to have first hook, first simple functionality like validation and then the rest will be easier and others will know how to contribute further |
Working on it!! 😛 |
I was thinking of an approach if we were going to use meow for parsing CLI commands #!/usr/bin/env node
import React from 'react';
import {render} from 'ink';
import meow from 'meow';
import App from './ui';
import Generate from './generate';
const cli = meow(`
Usage
$ generate <asyncapi> <template>
Options
--name Your name
Examples
$ learnINK --name=Jane
Hello, Jane
`, {
flags: {
name: {
type: 'string'
}
}
});
let [command, ...args] = cli.input;
switch(command) {
case "generate":
render(<Generate file={cli.input[1]} />);
} We can make different
|
yeah @Souvikns!! that's the idea, this is one of the steps that will drive to the approach you mention 😋 Thanks for the feedback!! 🤗 |
so what is missing, when do you feel comfortable to get some code review. I guess that just some cleanup is needed and we can also slowly start splitting the initial issue into separate issues for separate detaild discussion (if needed). I want |
@jotamusik I have a doubt about the argument parser commander when I use Usage: asyncapi [options] [command]
Options:
-V, --version output the version number
-h, --help display help for command
Commands:
generate [options] <asyncapi> <template> and when I use Usage: asyncapi generate [options] <asyncapi> <template>
Options:
-i, --install installs the template and its dependencies (defaults to false)
-h, --help display help for command It generates help option for all the commands , this helps users as different commands have different options that it supports. meow when I use Usage
$ generate <asyncapi> <template>
Options
-V, --version output the version number
-h, --help display help for command and when I use {
input: [ 'generate' ],
flags: { help: true },
unnormalizedFlags: { help: true },
pkg: {
name: 'mew',
version: '1.0.0',
description: '',
main: 'index.js',
scripts: { test: 'echo "Error: no test specified" && exit 1' },
keywords: [],
author: '',
license: 'ISC',
dependencies: { 'cli-meow-help': '^2.0.2', commander: '^7.2.0', meow: '^9.0.0' },
readme: 'ERROR: No README data found!',
_id: 'mew@1.0.0'
},
help: '\n' +
' Usage\n' +
' $ generate [options] <asyncapi> <template>\n' +
'\n' +
' Options\n' +
' -V, --version output the version number\n' +
' -h, --help display help for command\n',
showHelp: [Function: showHelp],
showVersion: [Function: showVersion]
} we can create our own custom help string for each commands though, but is there a way of doing this automatically in meow. |
As far as I know, we can't do this with meow. I maybe wrong though. But this makes me wonder, should we use the What are your thoughts? @jotamusik @derberg @Souvikns |
Looks like meow want's to support autogeneration of help -> sindresorhus/meow#80 no strong opinion for commander or yargs instead of meow. Gatsby CLI uses for example ink with yargs 🤷🏼 @aayushmau5 have a look at #1 to see how we want to design commands |
I was playing with meow a little bit and I think we can create our own help functions and make it just like commander from checking the command and the help flag const meow = require('meow');
const mewHelp = require('cli-meow-help');
const cli = meow(`
Usage
$ generate [options] <asyncapi> <template>
Options
-V, --version output the version number
-h, --help display help for command
`);
const commands = {
"generate": `
Usage
asyncapi generate [options] <asyncapi> <template>
Options
-i, --install installs the template and its dependencies
-h, --help display help for command
`
}
const helperText = (command, flag) => {
if(commands[command] && flag){
console.log(commands[command])
}
}
helperText(cli.input[0], cli.flags.help) Also found this library https://www.npmjs.com/package/cli-meow-help, it uses chalk to create a good-looking helper text for commander but it has a problem that we can't turn of any sections like even if we don't have any commands it will still render the command section and leave it empty. |
make it more abstract, for future development of another type of specification definition (maybe from a http server)
@jotamusik hey, where are we with this one. Do you need some help? Just want to make sure you are not trying to be too perfect with first PR. Keep in mind we can improve further and once we get this one merged we will also enable more people to help and improve further. |
Hi! @derberg I'm working on a first minimal feature of validation, so still WIP without trying to be perfect 😅 I know it's more time than expected, but it's because I have been busy the past weeks |
I'm having some issues with the tests of the validation component and I think that it's related with the useEffect() and the component lifecycle. If anyone can take a look, it would be great! 😛 |
The |
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.
@jotamusik I left a follow up comments in previous comments that were not answered
…dFileSync option param
…e it does not come with detail or validationErrors properties
…ted 'missing-asyncapi-field')
I think that we are done for this first version!! 🤔 |
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.
few minors but need to be fixed before the merge
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.
LGTM 🚀
@jotamusik Congrats!!! Now let us see if we did not forget about anything for release automation 🤞🏼 hold my 🍺
🎉 This PR is included in version 0.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…asyncapi#5) * Add new method to unify logic so metrics are collected when any command is invoked * Remove some unnecessary code * Change NR license key * Fix 'init()' to return a proper value for the command invoked * Unify logic for the remaining commands * Unify logic also for 'action.executed' metric * Rollback last changes
Description
Related issue(s)