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): implement latest version #76

Merged
merged 9 commits into from
Nov 26, 2022
Merged

Conversation

aldy505
Copy link
Member

@aldy505 aldy505 commented Nov 24, 2022

Closes #75

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

codecov bot commented Nov 24, 2022

Codecov Report

Base: 69.14% // Head: 69.35% // Increases project coverage by +0.20% 🎉

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
+ Coverage   69.14%   69.35%   +0.20%     
==========================================
  Files          12       12              
  Lines         752      757       +5     
  Branches       54       55       +1     
==========================================
+ Hits          520      525       +5     
  Misses        201      201              
  Partials       31       31              
Flag Coverage Δ
auth 32.24% <ø> (ø)
rce 92.58% <100.00%> (+0.10%) ⬆️
sdk-go 66.48% <ø> (ø)

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

Impacted Files Coverage Δ
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.

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.

btw, cara testingnya emang gitu ya? copas di tiap test casenya

rce/src/RceService.ts Outdated Show resolved Hide resolved
rce/src/runtime/acquire.ts Outdated Show resolved Hide resolved
rce/src/runtime/acquire.ts Show resolved Hide resolved
rce/src/runtime/acquire.ts Outdated Show resolved Hide resolved
Comment on lines 105 to 111
const runtime = new Runtime(
configObject.language,
configObject.version,
false,
configObject.extension,
configObject.compiled,
configObject.build_command,
Copy link
Member

Choose a reason for hiding this comment

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

ini keknya butuh diubah biar constructornya terima object, biar masing masing parameternya bisa dikasih nama, kalo banyak gini jadi bingung
tapi nanti aja deh, biar aku aja yg bikin PRnya

rce/src/runtime/acquire.ts Show resolved Hide resolved
rce/src/runtime/acquire.ts Outdated Show resolved Hide resolved
@@ -13,7 +14,7 @@ export class Runtime {
public readonly processLimit: number,
public readonly allowedEntrypoints: number
) {
if (language === "" || version === "" || extension === "" || runCommand.length === 0 || aliases.length === 0 || typeof environment !== "object" || allowedEntrypoints === 0) {
if (language === "" || version === "" || latest === undefined || extension === "" || runCommand.length === 0 || aliases.length === 0 || typeof environment !== "object" || allowedEntrypoints === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

ini juga rasanya butuh zod, tapi ntar aku aja deh yg bikin PRnya

aldy505 and others added 4 commits November 26, 2022 09:24
Co-authored-by: Dicha Zelianivan Arkana <dicha.arkana03@gmail.com>
Co-authored-by: Dicha Zelianivan Arkana <dicha.arkana03@gmail.com>
Co-authored-by: Dicha Zelianivan Arkana <dicha.arkana03@gmail.com>
Co-authored-by: Dicha Zelianivan Arkana <dicha.arkana03@gmail.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 26, 2022

This pull request introduces 2 alerts when merging 78b0fed into d6fb474 - view on LGTM.com

new alerts:

  • 2 for Syntax error

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed 🚀. For more information, please check out our post on the GitHub blog.

@elianiva elianiva merged commit 9a4d417 into master Nov 26, 2022
@elianiva elianiva deleted the feat/rce-latest-version branch November 26, 2022 10:12
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
Status: Done
Development

Successfully merging this pull request may close these issues.

Support specifying latest as version value on execute endpoint
2 participants