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

Switch package manager to npm #681

Conversation

Shofiya2003
Copy link

What kind of change does this PR introduce?
The PR includes changes to convert Talawa-Admin to use the NPM package manager instead of yarn.

Issue Number:

Fixes #546

Did you add tests for your changes?
No

Snapshots/Videos:

If relevant, did you update the documentation?
The documentation has been changed to replace the steps that involve yarn with NPM
The following .md files have been changed:
https://github.com/PalisadoesFoundation/talawa-admin/blob/develop/CONTRIBUTING.md
https://github.com/PalisadoesFoundation/talawa-admin/blob/develop/INSTALLATION.md
https://github.com/PalisadoesFoundation/talawa-admin/blob/develop/PR_GUIDELINES.md

Summary
This PR solves the problem which arose due to yarn v1. To maintain consistency with Talawa API, we switched the package manager to NPM.
Problems due to yarn v1
#493 (comment)

Does this PR introduce a breaking change?
No

Other information

Have you read the contributing guide?
Yes

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.

@Shofiya2003
Copy link
Author

Please suggest changes, if any.

@Cioppolo14
Copy link

@Shofiya2003 Please fix the failing tests. Our test code coverage system will fail if either of these two conditions occur:

The overall code coverage drops below the target threshold of the repository
Any file in the pull request has code coverage levels below the repository threshold

We do this to ensure the overall reliability of the code base is maintained. Thank you!

@palisadoes
Copy link
Contributor

1.Please check the pull-request.yml and push.ymlfiles in the .github/workflows directory

  1. We use them as part of the testing regime. They have references to yarn in them

@Shofiya2003
Copy link
Author

I changed all the references to yarn in those files

@Shofiya2003
Copy link
Author

@palisadoes @Cioppolo14
reduxjs/react-redux#1886
This error occurs when the workflow runs npm run typecheck . We would need to upgrade the react version or use overrides
I'll get back with a solution that works.

@kayceeDev
Copy link
Contributor

I have a question, why do we need to change the package manager? most people prefer yarn and I will outline my reasons below:

Both Yarn and NPM are popular package managers in the JavaScript ecosystem, and they serve similar purposes. Yarn was created by Facebook and was introduced as an alternative to NPM. However, both have their own pros and cons. Here are some points to consider:

Yarn Pros:

  • Faster and more reliable installation and dependency management.
  • Offline caching of packages and parallelized downloads.
  • Lockfile is more deterministic.
  • Better support for workspaces and monorepos.

NPM Pros:

  • Comes bundled with Node.js, which means you don't need to install any additional software.
  • Provides better security features such as package vulnerability scanning and audit reports.
  • Has a larger community, which means more resources and support are available.
  • NPM supports automatic publishing to the NPM registry from GitHub repositories.

In conclusion, both Yarn and NPM have their own advantages, and choosing one over the other depends on your specific needs and preferences. If you're working with a mono repo or have large-scale projects with complex dependencies, Yarn might be a better choice. If security is a priority for you or you need to publish packages to the NPM registry from GitHub repositories, NPM might be a better choice.

My questions are, while we can change to yarn later, is this a non-trivial issue? Can't we look at more important issues like the dashboard redesign? Is it not more important to consider upgrading to a more important package like the react-router?

The Installation.md file is clear and concise and anyone who has node and npm can be able to migrate to yarn successfully.

If all these have been considered and we still want to move to npm, then I am ok with it. Else, yarn can do same thing as npm

@xoldd
Copy link

xoldd commented Mar 13, 2023

@kayceeDev If you're really talking about package manager pnpm takes the cake. It's way ahead of both npm and yarn in terms of features. But should we use it? I'm not so sure, contributers are having trouble just setting up node.js here. There's also many contributers who use windows, and imo windows has weird quirks related to global installation caching yarn and pnpm use.

I don't currently see any use for either yarn or pnpm, except for saving storage space. And I don't think that's something we should push on contributers since most of them are only familiar with npm.

@kayceeDev
Copy link
Contributor

kayceeDev commented Mar 13, 2023

@kayceeDev If you're really talking about package manager pnpm takes the cake. It's way ahead of both npm and yarn in terms of features. But should we use it? I'm not so sure, contributers are having trouble just setting up node.js here. There's also many contributers who use windows, and imo windows has weird quirks related to global installation caching yarn and pnpm use.

I don't currently see any use for either yarn or pnpm, except for saving storage space. And I don't think that's something we should push on contributers since most of them are only familiar with npm.

What I am asking is, is it really important, changing package manager? If we think it is then no problems.

@xoldd
Copy link

xoldd commented Mar 13, 2023

@kayceeDev Yes there is a problem. The project uses yarn v1. It has reached end of life.

@kayceeDev
Copy link
Contributor

@kayceeDev Yes there is a problem. The project uses yarn v1. It has reached end of life.

Hahaha. End of life indeed. So why shouldn't upgrade the docs to use recent yarn versions instead of a complete overhaul. Just a thought though. I like your humor between 😁

@xoldd
Copy link

xoldd commented Mar 13, 2023

@kayceeDev If the project can work with latest npm versions, there wouldn't be any problems running it with yarn or pnpm. If we're really going to take the storage space on contributer's local system into consideration I'd advice using pnpm instead of yarn.

You see this I gave this issue 10 points as it's not as easy as it looks. Try changing the package manager or using yarn 3 on your local system with talawa-admin, you'll know what I'm talking about.

@kayceeDev
Copy link
Contributor

@kayceeDev If the project can work with latest npm versions, there wouldn't be any problems running it with yarn or pnpm. If we're really going to take the storage space on contributer's local system into consideration I'd advice using pnpm instead of yarn.

You see this I gave this issue 10 points as it's not as easy as it looks. Try changing the package manager or using yarn 3 on your local system with talawa-admin, you'll know what I'm talking about.

I totally get you. It's fine with your decision. I am just being open minded to these changes. Thank you for indulging me.

@github-actions
Copy link

This pull request did not get any activity in the past 10 days and will be closed in 365 days if no update occurs. Please verify it has no conflicts with the develop branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work.

@github-actions github-actions bot added the no-pr-activity No pull request activity label Mar 24, 2023
@palisadoes palisadoes requested a review from xoldd March 24, 2023 01:15
@palisadoes
Copy link
Contributor

@Shofiya2003 Please update the PR with the latest code changes. This should fix the failing type error tests.

@github-actions github-actions bot removed the no-pr-activity No pull request activity label Mar 25, 2023
@palisadoes
Copy link
Contributor

Closing as the issue has been reassigned

@palisadoes palisadoes closed this Mar 25, 2023
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.

Convert Talawa-Admin to use the NPM package manager
5 participants