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

Use PNPM #1088

Merged
merged 21 commits into from
May 8, 2024
Merged

Use PNPM #1088

merged 21 commits into from
May 8, 2024

Conversation

KhudaDad414
Copy link
Member

@KhudaDad414 KhudaDad414 commented Apr 27, 2024

Description

  • Use PNPM in test workflow
  • Use PNPM in the deploy workflow
  • Remove nodejs tag from the repo after merge

Copy link

changeset-bot bot commented Apr 27, 2024

🦋 Changeset detected

Latest commit: b5d49c3

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

netlify bot commented Apr 27, 2024

Deploy Preview for asyncapi-studio-design-system ready!

Name Link
🔨 Latest commit b5d49c3
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-studio-design-system/deploys/663a4ab5f959e200093be4ee
😎 Deploy Preview https://deploy-preview-1088--asyncapi-studio-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 27, 2024

Deploy Preview for studio-next ready!

Name Link
🔨 Latest commit b5d49c3
🔍 Latest deploy log https://app.netlify.com/sites/studio-next/deploys/663a4ab5a2de400009e51604
😎 Deploy Preview https://deploy-preview-1088--studio-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 27, 2024

Deploy Preview for modest-rosalind-098b67 ready!

Name Link
🔨 Latest commit b5d49c3
🔍 Latest deploy log https://app.netlify.com/sites/modest-rosalind-098b67/deploys/663a4ab56af6290008c14a1f
😎 Deploy Preview https://deploy-preview-1088--modest-rosalind-098b67.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@KhudaDad414 KhudaDad414 marked this pull request as draft April 28, 2024 00:31
@KhudaDad414 KhudaDad414 marked this pull request as ready for review April 28, 2024 00:57
@KhudaDad414
Copy link
Member Author

KhudaDad414 commented May 1, 2024

@Amzani @Shurtu-gal. PR is ready to review.

PS: Sorry for the long PR. It was like opening a can of bugs (pun intended).

@Shurtu-gal
Copy link
Collaborator

@KhudaDad414 there are some conflicts

@KhudaDad414
Copy link
Member Author

@Shurtu-gal the conflicts has been resolved.

@KhudaDad414
Copy link
Member Author

/ptal

@asyncapi-bot
Copy link
Contributor

@Amzani @magicmatatjahu @princerajpoot20 Please take a look at this PR. Thanks! 👋

Copy link
Collaborator

Choose a reason for hiding this comment

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

@KhudaDad414 could we not have changes in GitHub workflows as it is something managed from .github repo.

Copy link
Member Author

@KhudaDad414 KhudaDad414 May 7, 2024

Choose a reason for hiding this comment

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

unfortunately we can't. .github only supports npm.

if everything goes well, we can purpose our changes to .github repo and make sure other packages in AsyncAPI can also use pnpm as their package manager.

I have also removed the nodejs tag which makes sure that we do not get updates for the test script.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's fine I was only speaking about this specific file but as it is the same with the .github one there is no issue. I guess I was seeing commit diff leading to this file being shown as modified.

Copy link
Collaborator

@Amzani Amzani left a comment

Choose a reason for hiding this comment

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

Looks good, only small change regarding this sentence in the README

Note: NPM v7+ is required.

Change to pnpm

@KhudaDad414
Copy link
Member Author

@Amzani done.

@KhudaDad414 KhudaDad414 requested a review from Amzani May 7, 2024 15:37
Copy link

sonarqubecloud bot commented May 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Collaborator

@Amzani Amzani left a comment

Choose a reason for hiding this comment

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

LGTM

@KhudaDad414
Copy link
Member Author

/rtm

@asyncapi-bot asyncapi-bot merged commit 530468f into asyncapi:master May 8, 2024
23 checks passed
@KhudaDad414 KhudaDad414 deleted the use-pnpm branch May 15, 2024 11:18
KhudaDad414 added a commit to KhudaDad414/studio that referenced this pull request Jun 4, 2024
Co-authored-by: asyncapi-bot <[email protected]>
@derberg
Copy link
Member

derberg commented Jul 17, 2024

/help

@asyncapi-bot
Copy link
Contributor

Hello, @derberg! 👋🏼

    I'm 🧞🧞🧞 Genie 🧞🧞🧞 from the magic lamp. Looks like somebody needs a hand!

    At the moment the following comments are supported in pull requests:

    - `/please-take-a-look` or `/ptal` - This comment will add a comment to the PR asking for attention from the reviewrs who have not reviewed the PR yet.
    - `/ready-to-merge` or `/rtm` - This comment will trigger automerge of PR in case all required checks are green, approvals in place and do-not-merge label is not added
    - `/do-not-merge` or `/dnm` - This comment will block automerging even if all conditions are met and ready-to-merge label is added
    - `/autoupdate` or `/au` - This comment will add `autoupdate` label to the PR and keeps your PR up-to-date to the target branch's future changes. Unless there is a merge conflict or it is a draft PR. (Currently only works for upstream branches.)
    - `/update` or `/u` - This comment will update the PR with the latest changes from the target branch. Unless there is a merge conflict or it is a draft PR. NOTE: this only updates the PR once, so if you need to update again, you need to call the command again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants