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

feat(rce): support multiple files #63

Merged
merged 15 commits into from
Nov 13, 2022
Merged

feat(rce): support multiple files #63

merged 15 commits into from
Nov 13, 2022

Conversation

aldy505
Copy link
Member

@aldy505 aldy505 commented Nov 10, 2022

Closes #62

@aldy505 aldy505 added enhancement New feature or request module: rce labels Nov 10, 2022
@aldy505 aldy505 self-assigned this Nov 10, 2022
@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Base: 67.00% // Head: 69.87% // Increases project coverage by +2.87% 🎉

Coverage data is based on head (ed18370) compared to base (88e3b8f).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #63      +/-   ##
==========================================
+ Coverage   67.00%   69.87%   +2.87%     
==========================================
  Files          11       12       +1     
  Lines         700      737      +37     
  Branches       48       54       +6     
==========================================
+ Hits          469      515      +46     
+ Misses        199      191       -8     
+ Partials       32       31       -1     
Flag Coverage Δ
auth 32.16% <ø> (ø)
rce 92.47% <100.00%> (+3.65%) ⬆️
sdk-go 66.48% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
rce/src/job/files.ts 100.00% <100.00%> (ø)
rce/src/job/job.ts 89.69% <100.00%> (+4.26%) ⬆️
rce/src/runtime/runtime.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2022

This pull request introduces 1 alert when merging 58dffd3 into 88e3b8f - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@aldy505 aldy505 marked this pull request as ready for review November 13, 2022 07:46
Copy link
Member

@elianiva elianiva left a comment

Choose a reason for hiding this comment

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

cuma stylistic changes

rce/src/RceService.ts Outdated Show resolved Hide resolved
Comment on lines -147 to -166
const missingParameters: string[] = [];
if (req.body?.language === undefined || req.body?.language === null || typeof req.body.language !== "string" || req.body.language === "") {
missingParameters.push("language");
}

if (req.body?.version === undefined || req.body?.version === null || typeof req.body?.version !== "string" || req.body.version === "") {
missingParameters.push("version");
}

if (req.body?.code === undefined || req.body?.code === null || typeof req.body?.code !== "string" || req.body.code === "") {
missingParameters.push("code");
}

if (req.body?.compileTimeout !== undefined && req.body?.compileTimeout !== null && (typeof req.body.compileTimeout !== "number" || req.body.compileTimeout > 30_000)) {
missingParameters.push("compileTimeout");
}

if (req.body?.runTimeout !== undefined && req.body?.runTimeout !== null && (typeof req.body.runTimeout !== "number" || req.body.runTimeout > 30_000)) {
missingParameters.push("runTimeout");
}
Copy link
Member

Choose a reason for hiding this comment

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

puas banget lihat ini

rce/src/job/files.ts Outdated Show resolved Hide resolved
code: string,
entrypoint: boolean
}

/**
* @generated from protobuf message rce.CodeRequest
Copy link
Member

Choose a reason for hiding this comment

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

ini file kalo udah bukan hasil codegen mending nanti rapihin aja deh sekalian wkwkw
tapi nanti aja beda PR

Copy link
Member Author

Choose a reason for hiding this comment

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

nanti aja ini mah. masih ada yang hasil codegen juga

aldy505 and others added 2 commits November 13, 2022 17:23
Co-authored-by: Dicha Zelianivan Arkana <dicha.arkana03@gmail.com>
Co-authored-by: Dicha Zelianivan Arkana <dicha.arkana03@gmail.com>
@aldy505 aldy505 merged commit 86a1c12 into master Nov 13, 2022
@aldy505 aldy505 deleted the feat/multiple-files branch November 13, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request module: rce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multi file execution support
2 participants