Skip to content

Use nodejs scripts instead of sed to modify some imports #1181

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.

Sign up for GitHub

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

Jump to bottom
Merged
pipecd-bot merged 2 commits into from
Dec 2, 2020
Merged

Use nodejs scripts instead of sed to modify some imports #1181

pipecd-bot merged 2 commits into from
Dec 2, 2020

Conversation

@cakecatz
Copy link
Contributor

@cakecatz cakecatz commented Dec 2, 2020 •
edited
Loading

What this PR does / why we need it:

Use JS to strip unnecessary imports from generated files(:api_client, :models).
I think it's better than using sed because nodejs_binary is added as Bazel dependency(sed is not).

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

@pipecd-bot pipecd-bot requested a review from nghialv December 2, 2020 09:38
@cakecatz cakecatz changed the title Use nodejs scripts instead of sed to strip some imports Use nodejs scripts instead of sed to modify some imports Dec 2, 2020
khanhtc1202
khanhtc1202 reviewed Dec 2, 2020
View reviewed changes
khanhtc1202
khanhtc1202 reviewed Dec 2, 2020
View reviewed changes
@pipecd-bot
Copy link
Collaborator

pipecd-bot commented Dec 2, 2020

COVERAGE

Code coverage for golang is 33.28%. This pull request does not change code coverage.

@pipecd-bot
Copy link
Collaborator

pipecd-bot commented Dec 2, 2020

COVERAGE

Code coverage for javascript is 83.14%. This pull request does not change code coverage.

@pipecd-bot
Copy link
Collaborator

pipecd-bot commented Dec 2, 2020

COVERAGE

Code coverage for golang is 33.28%. This pull request does not change code coverage.

@pipecd-bot
Copy link
Collaborator

pipecd-bot commented Dec 2, 2020

COVERAGE

Code coverage for javascript is 83.14%. This pull request does not change code coverage.

@khanhtc1202
Copy link
Member

khanhtc1202 commented Dec 2, 2020

/approve

@pipecd-bot
Copy link
Collaborator

pipecd-bot commented Dec 2, 2020

APPROVE

This pull request is APPROVED by khanhtc1202.

Approvers can cancel the approval by writing /approve cancel in a comment. Any additional commits also will change this pull request to be not-approved.

@pipecd-bot pipecd-bot merged commit 49592a3 into master Dec 2, 2020
@pipecd-bot pipecd-bot deleted the fix-web-build branch December 2, 2020 11:02
@cakecatz cakecatz mentioned this pull request Dec 3, 2020
Merged
pipecd-bot pushed a commit that referenced this pull request Dec 3, 2020
@cakecatz
Update eslintrc (#1182)
26e7934 
**What this PR does / why we need it**:

* exclude js files from `@typescript-eslint/no-var-requires`

Ref: #1181 (comment)

**Which issue(s) this PR fixes**:



**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
-->
```release-note
NONE
```

This PR was merged by Kapetanios.
@pipecd-bot pipecd-bot mentioned this pull request Dec 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@khanhtc1202 khanhtc1202 khanhtc1202 left review comments

@nghialv nghialv Awaiting requested review from nghialv

Assignees

No one assigned

Labels

approved area/web release-note-none size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cakecatz @pipecd-bot @khanhtc1202