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(info): adding more functionality to info package #833
Conversation
a small step towards improving the current info package
for faster and precise execution, a list of supported flags is added
Now that you're proposing some new flags, which are Great, we should also provide an help command/flag so we show how to use 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.
Can you also include your .lock
file? That should fix the tests. 👍
updates the lock file of packages/info
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.
Seems to be a good idea to me :) left a change for you.
Also, add [WIP] in the PR title as work is not completed here IMO
@ematipico check this out.
Output formats
Default |
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.
Gonna take this for a spin on a corp project, so chilling this for a bit
This one is really great . You can give the command name like |
@evenstensberg I am thinking of going with ASTs to extract the required information from the config file. Is there any other way around 😎 |
@evenstensberg why did you chill a PR that is still WIP? |
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.
A suggestion
adds more options and command types
fixed the linting error
@@ -0,0 +1,13 @@ | |||
export const AVAILABLE_COMMANDS: string[] = [ |
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.
You use these constants inside your commands.ts, so you can keep your code DRY.
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.
I am not getting it.
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.
- You should create an
enum
for your available commands. - Later you will use AVAILABLE_COMMANDS.BINARIES inside
commands.ts
Basically it's not convenient to have the same string repeated in different places. Create ONE constant and it in those places
} | ||
export default async function info(CustomArgv: object): Promise<void> { | ||
const CUSTOM_AGRUMENTS: boolean = typeof CustomArgv === "object"; | ||
const args: ArgvI = CUSTOM_AGRUMENTS ? CustomArgv : argv; |
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.
You define the options using yargs, so could you please use yargs to parse the arguments too? It will keep consistency. Sorry to be annoying but I don't see any reason to not do that
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 more suggestions
updates the docs and the package description
removed comments
@pranshuchittora Thanks for your update. I labeled the Pull Request so reviewers will review it again. @rishabh3112 Please review the new changes. |
yarn add @webpack-cli/info -D | ||
|
||
#npx | ||
npx webpack info [options] |
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.
It's not an installation command, it should be under usage :)
now plugin name can be retrived from the constructor call
@pranshuchittora Please review the following output log for errors:
See complete report here. |
Could you rebase this against the next branch? |
Refer #1007 |
Today, while working on this PR, I observed that many CLI
packages
have very fewer downloads.info
package downloadsAs a developer, I have expected the
info
package to give some information about config or something related to webpack. Strangely it outputs some environment information.So the idea is to revamp the current
info
package by adding a few features.Expected behaviour
Solution: By using the power of AST we can parse the config file & get the required details.
The current env info looks good, but someone might want some other env info, so the idea is to add more env info & access them dynamically by flags.
Multiple flags supported, in order to get multiple env info
Solution: By reading the process arguments (flags)
What kind of change does this PR introduce?
Feature
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
Yes
I am working on this, so pls suggest any improvements. 🙏
Let's discuss 🔥