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

feat: pm-install generator #9

Merged
merged 31 commits into from
Feb 6, 2024
Merged

feat: pm-install generator #9

merged 31 commits into from
Feb 6, 2024

Conversation

cpreston321
Copy link
Member

@cpreston321 cpreston321 commented Feb 6, 2024

πŸ”— Linked issue

#5

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

# ✨ Auto-detect
npx nypm@latest add automd-playground

# npm
npm install automd-playground

# yarn
yarn add automd-playground

# pnpm
pnpm install automd-playground

# bun
bun install automd-playground

Args:
- name? (falls back to package.json "name")
- dev? (if true adds -D or --save-dev)
- auto? (if false remove nypm)

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@cpreston321 cpreston321 requested a review from pi0 February 6, 2024 19:43
@cpreston321 cpreston321 changed the title feat: 'npm-install' generator feat: npm-install generator Feb 6, 2024
playground/README.md Outdated Show resolved Hide resolved
src/generators/npm-install.ts Outdated Show resolved Hide resolved
src/generators/npm-install.ts Outdated Show resolved Hide resolved
@pi0
Copy link
Member

pi0 commented Feb 6, 2024

Do you mind to also add a new section to readme? πŸ™πŸΌ

@cpreston321 cpreston321 changed the title feat: npm-install generator feat: pm-install generator Feb 6, 2024
@cpreston321 cpreston321 requested a review from pi0 February 6, 2024 20:15
playground/package.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@cpreston321
Copy link
Member Author

@pi0 I added the changes for nypm.

README.md Outdated Show resolved Hide resolved
@uncenter
Copy link
Collaborator

uncenter commented Feb 6, 2024

Would support for npx/bunx/pnpm dlx be a separate PR? Something like pm-run? Happy to implement that unless @cpreston321 wants to :)

@cpreston321
Copy link
Member Author

Would support for npx/bunx/pnpm dlx be a separate PR? Something like pm-run? Happy to implement that unless @cpreston321 wants to :)

I would submit a new issue and talk to @pi0 about the feature!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cpreston321 and others added 4 commits February 6, 2024 17:04
Co-authored-by: uncenter <47499684+uncenter@users.noreply.github.com>
Co-authored-by: uncenter <47499684+uncenter@users.noreply.github.com>
Co-authored-by: uncenter <47499684+uncenter@users.noreply.github.com>
Co-authored-by: uncenter <47499684+uncenter@users.noreply.github.com>
@uncenter uncenter mentioned this pull request Feb 6, 2024
2 tasks
pi0 and others added 2 commits February 6, 2024 23:19
Co-authored-by: uncenter <47499684+uncenter@users.noreply.github.com>
Co-authored-by: uncenter <47499684+uncenter@users.noreply.github.com>

#### Args supported for `pm-install`

- `name`: The package name (by default tries to read from the `name` field in `package.json`).
Copy link
Collaborator

@uncenter uncenter Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on packages instead of name? I've often seen READMEs that suggest installing multiple at once (e.g. typescript-eslint suggests installing typescript as well).

For multiple, it could be a comma-separated (and space friendly) string:

    <!-- AUTOMD_START generator="pm-install" packages="package-name, other-package-name" dev="true" -->

    ```sh
    # npm
    npm install -D package-name other-package-name

    # yarn
    yarn add -D package-name other-package-name

    # pnpm
    pnpm install -D package-name other-package-name

    # bun
    bun install -D package-name other-package-name
    ```

    <!-- AUTOMD_END -->

And for a single one it would look the same but with a different key:

    <!-- AUTOMD_START generator="pm-install" packages="package-name" dev="true" -->

    ```sh
    # npm
    npm install -D package-name

    # yarn
    yarn add -D package-name

    # pnpm
    pnpm install -D package-name

    # bun
    bun install -D package-name
    ```

    <!-- AUTOMD_END -->

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yes but it is also less common and I prefer to keep name shorter pm already suggests it is about package. We can easily also use name="pkg1 pkg2" btw even without array support wdyt?

Copy link
Collaborator

@uncenter uncenter Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well pm stands for "package manager" right? So it doesn't really suggest one way or another. How about we make it space separated though, you are right that comma-separated is a little much.

    <!-- AUTOMD_START generator="pm-install" packages="pkg1 pkg2" -->
    
    <!-- AUTOMD_START generator="pm-install" packages="pkg" -->

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i read pm-install packages= as package manager install packages="..." while pm-install name= as package manger install name=.

More precisely what are setting in this param (according to npm) is "package specs"

Although we can really simply support packages as an alias if you strongly like it more feel free to make a new PR to propose (thinking to move this one forward faster)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good yeah, I'll make a PR in the future!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
pi0 and others added 3 commits February 6, 2024 23:41
Co-authored-by: uncenter <47499684+uncenter@users.noreply.github.com>
@pi0 pi0 merged commit c4bdecb into main Feb 6, 2024
2 checks passed
@pi0
Copy link
Member

pi0 commented Feb 6, 2024

Thank you both @cpreston321 and @uncenter for your contribution it was nothing I expected and happy having your collaborations ❀️

Mind you both automd and omark projects are really in trial stages and I usually used to keep projects like these private. So nothing in them is set in stone/ we don't need to worry about small details we can fix them later once main goals (like this generator) were met πŸ‘πŸΌ

@pi0 pi0 deleted the feat/npmInstallGen branch February 6, 2024 22:44
@cpreston321 cpreston321 mentioned this pull request Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants