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

Use perl instead of sed to fix build error in macos. #5700

Merged
merged 1 commit into from
Dec 3, 2020

Conversation

theSinner
Copy link
Contributor

Hi

In my previous PR for adding change language option, @floreks added some lines to aio/scripts/build.sh to prevent caching after changing language using sed command which makes build failed in macOS, because in macOS sed command is a little bit different from Linux.
I changed sed with perl which does the same thing and the dashboard builds successfully in macOS and I think it works in Linux, too.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 2, 2020
@codecov
Copy link

codecov bot commented Dec 2, 2020

Codecov Report

Merging #5700 (95e095d) into master (5cb109f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5700   +/-   ##
=======================================
  Coverage   44.36%   44.36%           
=======================================
  Files         214      214           
  Lines        9124     9124           
  Branches      113      113           
=======================================
  Hits         4048     4048           
  Misses       4801     4801           
  Partials      275      275           

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 5cb109f...95e095d. Read the comment docs.

@maciaszczykm
Copy link
Member

maciaszczykm commented Dec 3, 2020

It's true, that macOS has different version of sed built-in and it doesn't work with current build script. Normally, I'd recommend to install GNU version of sed but if pearl will work on Linux too I think we can merge it to make it more convenient for all of us.

@floreks can you verify if it works on Linux? After change there are no more issues on macOS, so I'd be glad to merge it.

@floreks
Copy link
Member

floreks commented Dec 3, 2020

I will verify and merge if it works.

@floreks
Copy link
Member

floreks commented Dec 3, 2020

Seems to be working just fine on Ubuntu.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floreks, theSinner

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 3, 2020
@k8s-ci-robot k8s-ci-robot merged commit 16c2735 into kubernetes:master Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants