Skip to content

Maintenance: npm script package is incorrect #1343

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

Closed
Muthuveerappanv opened this issue Feb 28, 2023 · 4 comments · Fixed by #1344
Closed

Maintenance: npm script package is incorrect #1343

Muthuveerappanv opened this issue Feb 28, 2023 · 4 comments · Fixed by #1344
Assignees
Labels
completed This item is complete and has been merged/shipped internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)

Comments

@Muthuveerappanv
Copy link
Contributor

Expected Behaviour

the main package.json has a script package.

Excepted behavior is that it will package the entire workspace.

Current Behaviour

Currently it goes in a recursive loop

> [email protected] package
> npm run package


> [email protected] package
> npm run package


> [email protected] package
> npm run package


> [email protected] package
> npm run package


> [email protected] package
> npm run package

Code snippet

npm run package

Steps to Reproduce

on the root directory of aws-lambda-powertools-typescript

run

npm install
npm run package

Possible Solution

No response

AWS Lambda Powertools for TypeScript version

latest

AWS Lambda function runtime

18.x

Packaging format used

npm

Execution logs

No response

@Muthuveerappanv Muthuveerappanv added triage This item has not been triaged by a maintainer, please wait bug Something isn't working labels Feb 28, 2023
Muthuveerappanv added a commit to Muthuveerappanv/aws-lambda-powertools-typescript that referenced this issue Feb 28, 2023
@dreamorosi dreamorosi changed the title Bug: npm script package is incorrect Maintenance: npm script package is incorrect Feb 28, 2023
@dreamorosi dreamorosi added internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) confirmed The scope is clear, ready for implementation and removed bug Something isn't working triage This item has not been triaged by a maintainer, please wait labels Feb 28, 2023
@dreamorosi
Copy link
Contributor

dreamorosi commented Feb 28, 2023

Hi Muthu, thank you for opening this issue.

You are right, in its current form the command is broken and causes a recursion. I think the issue went unnoticed because we don't really use the command as we use lerna to package (for now) and all other commands that are run across the workspace in our CI are always ran with the proper flag to make more explicit what is being run.

I am also changing the categorization of the issue from bug to maintenance as the issue doesn't affect runtime behavior (read is this a bug?).

Nevertheless, I think for the time being we should fix this.


Note: I see that you have already opened a PR, thank you for the bias for action. Next time however, please consider waiting until one of the maintainers has triaged the issue before moving onto a PR.
We try to follow the process described in the contributing guidelines (here) to make sure that issues have been discussed and their scope has been agreed upon to minimize the risk of investing time on a PR that might not be valid or not get the attention it deserves (in terms of reviews) on a timely manner. Appreciate your understanding!

@Muthuveerappanv
Copy link
Contributor Author

Hi Muthu, thank you for opening this issue.

You are right, in its current form the command is broken and causes a recursion. I think the issue went unnoticed because we don't really use the command as we use lerna to package (for now) and all other commands that are run across the workspace in our CI are always ran with the proper flag to make more explicit what is being run.

I am also changing the categorization of the issue from bug to maintenance as the issue doesn't affect runtime behavior (read is this a bug?).

Nevertheless, I think for the time being we should fix this.

Note: I see that you have already opened a PR, thank you for the bias for action. Next time however, please consider waiting until one of the maintainers has triaged the issue before moving onto a PR. We try to follow the process described in the contributing guidelines (here) to make sure that issues have been discussed and their scope has been agreed upon to minimize the risk of investing time on a PR that might not be valid or not get the attention it deserves (in terms of reviews) on a timely manner. Appreciate your understanding!

Thank you @dreamorosi, and appreciate the explanation of the PR & Issue process, point noted 👍
Will follow the same in the Issues & PRs to come!

@dreamorosi
Copy link
Contributor

Thank you for understanding and again, appreciate the help!

PR looks good now, I have approved it and will merge it later today.

dreamorosi pushed a commit that referenced this issue Feb 28, 2023
* fix worspace package script

fixes - #1343

* fix review comments, skip package for docs snippets

---------

Co-authored-by: Muthu Venkatachalam <[email protected]>
@github-project-automation github-project-automation bot moved this from Working on it to Coming soon in AWS Lambda Powertools for TypeScript Feb 28, 2023
@github-actions
Copy link
Contributor

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@github-actions github-actions bot added the pending-release This item has been merged and will be released soon label Feb 28, 2023
@dreamorosi dreamorosi moved this from Coming soon to Shipped in AWS Lambda Powertools for TypeScript Feb 28, 2023
@dreamorosi dreamorosi added completed This item is complete and has been merged/shipped and removed pending-release This item has been merged and will be released soon confirmed The scope is clear, ready for implementation labels Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed This item is complete and has been merged/shipped internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants