Skip to content

refactor: Enable triage process to run @serverless/compose#10950

Merged
pgrzesik merged 1 commit into
mainfrom
poc-compose-integration
Apr 14, 2022
Merged

refactor: Enable triage process to run @serverless/compose#10950
pgrzesik merged 1 commit into
mainfrom
poc-compose-integration

Conversation

@pgrzesik

@pgrzesik pgrzesik commented Apr 8, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Internal

@codecov

codecov Bot commented Apr 8, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #10950 (90e5169) into main (11b5e2a) will decrease coverage by 0.41%.
The diff coverage is 8.57%.

@@            Coverage Diff             @@
##             main   #10950      +/-   ##
==========================================
- Coverage   86.38%   85.96%   -0.42%     
==========================================
  Files         305      306       +1     
  Lines       12948    13018      +70     
==========================================
+ Hits        11185    11191       +6     
- Misses       1763     1827      +64
Impacted Files Coverage Δ
bin/serverless.js 47.05% <0.00%> (-4.01%) ⬇️
lib/cli/run-compose.js 0.00% <0.00%> (ø)
lib/cli/triage.js 90.62% <100.00%> (+0.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 11b5e2a...90e5169. Read the comment docs.

medikoo
medikoo reviewed Apr 8, 2022
View reviewed changes
Comment thread lib/cli/triage.js
)
).some(Boolean)
) {
return '@serverless/compose';

@medikoo medikoo Apr 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would follow convention, we have so far, where CLI name matches package name

Comment thread test/unit/lib/cli/triage/index.test.js Outdated
}
}

it('Should recognize "@serverless/compose" if "serverless-compose.yml" file present', async () => {

@medikoo medikoo Apr 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's enough to add CLI name to L60, and it then should be automatically tested (no need for other it)

@pgrzesik
pgrzesik force-pushed the poc-compose-integration branch from a53c955 to 471d296 Compare April 11, 2022 15:37
mnapoli
mnapoli reviewed Apr 11, 2022
View reviewed changes
Comment thread lib/cli/run-compose.js Outdated
@pgrzesik
pgrzesik force-pushed the poc-compose-integration branch from 471d296 to 971a161 Compare April 12, 2022 08:00
@mnapoli

mnapoli commented Apr 13, 2022

Copy link
Copy Markdown
Contributor

Just tried this branch again (removed previous Compose version from the project) and got this:

❯ sls-dev deploy

Serverless Compose needs to be installed first. This is a one-time operation.
? Do you want to install Serverless Compose locally with "npm"? Yes
/Users/matthieu/dev/serverless/serverless/bin/serverless.js:79
          throw error;
          ^

TypeError [ERR_INVALID_ARG_TYPE]: The "id" argument must be of type string. Received null
    at new NodeError (node:internal/errors:372:5)
    at validateString (node:internal/validators:120:11)
    at Module.require (node:internal/modules/cjs/loader:992:3)
    at require (node:internal/modules/cjs/helpers:102:18)
    at module.exports (/Users/matthieu/dev/serverless/serverless/lib/cli/run-compose.js:125:5) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Node.js v17.8.0

Compose was installed though:

{
  "devDependencies": {
    "@serverless/compose": "^0.0.11"
  }
}

@pgrzesik
pgrzesik force-pushed the poc-compose-integration branch from 971a161 to 6a3f221 Compare April 13, 2022 15:32
@pgrzesik

pgrzesik commented Apr 13, 2022

Copy link
Copy Markdown
Contributor Author

it should be fixed and working now @mnapoli

@mnapoli

mnapoli commented Apr 13, 2022

Copy link
Copy Markdown
Contributor

👍 all good

@pgrzesik
pgrzesik force-pushed the poc-compose-integration branch from 6a3f221 to 90e5169 Compare April 14, 2022 13:03
@pgrzesik
pgrzesik marked this pull request as ready for review April 14, 2022 14:54
@pgrzesik pgrzesik changed the title feat: Compose integration with automatic local install refactor: Enable triage process to run @serverless/compose Apr 14, 2022
@pgrzesik
pgrzesik merged commit 401c721 into main Apr 14, 2022
@pgrzesik
pgrzesik deleted the poc-compose-integration branch April 14, 2022 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@medikoo medikoo medikoo left review comments
@mnapoli mnapoli mnapoli left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pgrzesik @mnapoli @medikoo