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

Standardize use of scripts in package.json for common tasks #831

Merged
merged 20 commits into from
Sep 15, 2023

Conversation

MehulKChaudhari
Copy link
Member

Rendered text

Standardize the use of yarn and npm scripts in the Ember experience

This change encourages developers to use npm or yarn for certain commands when working with Ember applications, rather than using global Ember CLI commands like ember serve. This aligns Ember with norms in the JavaScript community and helps in reducing the confusion around Ember-specific commands.

text/0000-standardize-use-npm-yarn.md Outdated Show resolved Hide resolved
text/0000-standardize-use-npm-yarn.md Outdated Show resolved Hide resolved
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
> There are tradeoffs to choosing any path, please attempt to identify them here.

## Alternatives

Copy link
Contributor

@NullVoxPopuli NullVoxPopuli Jul 22, 2022

Choose a reason for hiding this comment

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

as an alternative, I would like all options presented all the time, (via global tab system) like how Microsoft and Starbeam do

example:
image

clicking the tab for pnpm/npm/yarn changes the tabs everywhere

from: https://starbeamjs.com/frameworks/react/1-getting-started.html

Copy link
Contributor

Choose a reason for hiding this comment

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

The learning team & friends are building in something like this in order to support TypeScript + JS in our docs. I would love to have all 3 here as well.

@jenweber
Copy link
Contributor

Just adding a note here that I will be the core team champion for this RFC.

Co-authored-by: Jen Weber <weberj10@gmail.com>
@chriskrycho chriskrycho added T-learning T-ember-cli RFCs that impact the ember-cli library labels Aug 4, 2022
@jenweber
Copy link
Contributor

@MehulKChaudhari @Dhanush027 Today the Framework team reviewed this RFC, and they had some suggestions to help make it more broadly applicable! Let me know if you want help making any of these changes:

  • Instead of focusing solely on npm start/test, we can revise the "motivation" section and title to be about using the scripts in package.json rather than focusing on npm/yarn. It is the scripts that are the industry standard we want to point to, not necessarily the CLI tool
  • We can mention the following commands as examples: test, start, build, and prepare
  • We should mention that developers should always refer to Contributing.md for full instructions when working with a new app or addon
  • We can include a link to "prior art" of showing npm start in CLI output. Here's when the text was introduced: ember-cli/ember-cli@6c50caa

After we make these revisions, we can communicate that this RFC is in FCP (final comment period) which will be a good opportunity to invite people to take another look, or a look for the first time.

@dhanushkumar-dev
Copy link

I'd like to help to make these changes to RFC.

@MehulKChaudhari
Copy link
Member Author

Yes I would like to help with point 2, 3, and 4

@bertdeblock
Copy link
Member

👋 Hi, wondering what the status is of this RFC?

Copy link
Contributor

@MrChocolatine MrChocolatine left a comment

Choose a reason for hiding this comment

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

Aesthetic suggestions.

text/0000-standardize-use-npm-yarn.md Outdated Show resolved Hide resolved
text/0000-standardize-use-npm-yarn.md Outdated Show resolved Hide resolved
text/0000-standardize-use-npm-yarn.md Outdated Show resolved Hide resolved
text/0000-standardize-use-npm-yarn.md Outdated Show resolved Hide resolved
text/0000-standardize-use-npm-yarn.md Outdated Show resolved Hide resolved
text/0000-standardize-use-npm-yarn.md Outdated Show resolved Hide resolved
MehulKChaudhari and others added 7 commits November 10, 2022 16:02
Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com>
Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com>
Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com>
Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com>
Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com>
Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com>
@wagenet wagenet added the S-Proposed In the Proposed Stage label Dec 2, 2022
@jenweber jenweber changed the title RFC: Standardize npm yarn use RFC RFC: Standardize package.json use RFC Dec 5, 2022
@MehulKChaudhari MehulKChaudhari changed the title RFC: Standardize package.json use RFC RFC: Standardize use of scripts in package.json for common tasks Dec 5, 2022
@wagenet wagenet added S-Exploring In the Exploring RFC Stage and removed S-Proposed In the Proposed Stage labels Jan 27, 2023
@locks
Copy link
Contributor

locks commented Sep 11, 2023

@MehulKChaudhari are you able to update the RFC so the checks pass? @jenweber can help you!

@locks locks self-assigned this Sep 11, 2023
@MehulKChaudhari
Copy link
Member Author

@MehulKChaudhari are you able to update the RFC so the checks pass? @jenweber can help you!

sure will do that

@jenweber
Copy link
Contributor

jenweber commented Sep 13, 2023

It looks like our linter does not correctly ignore new files in the images folder. Should we merge without a green linter for this one, assuming consensus for the RFC content? Comment out that linter? Someone could fix it real quick in another PR? I would like this to not be blocked by our tooling.

@locks
Copy link
Contributor

locks commented Sep 13, 2023

@kategengler do you have time to help out with the tooling?

@kategengler
Copy link
Member

@locks I am looking at the tooling. It used to ignore the images (really need to figure out automated tests for workflows)

@jenweber I think it is fine to merge over the failures if it has been verified the RFC does pass those checks.

@kategengler
Copy link
Member

@jenweber worth nothing you'll have to open the advancement PR yourself using https://github.com/emberjs/rfcs/blob/master/.github/READTHIS.md#trigger-opening-advancement-pryml

@kategengler kategengler changed the title RFC: Standardize use of scripts in package.json for common tasks Standardize use of scripts in package.json for common tasks Sep 15, 2023
@ef4 ef4 merged commit bd33242 into emberjs:master Sep 15, 2023
8 checks passed
ef4 added a commit that referenced this pull request Apr 19, 2024
Advance RFC #831 `"Standardize use of scripts in package.json for common tasks"` to Stage Released
ef4 added a commit that referenced this pull request Apr 26, 2024
Advance RFC #831 `"Standardize use of scripts in package.json for common tasks"` to Stage Recommended
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Final Comment Period S-Exploring In the Exploring RFC Stage T-ember-cli RFCs that impact the ember-cli library T-learning
Projects
None yet
Development

Successfully merging this pull request may close these issues.