Skip to content

Fix typo pipline -> pipeline #3698

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
merged 6 commits into from
Feb 27, 2018
Merged

Fix typo pipline -> pipeline #3698

merged 6 commits into from
Feb 27, 2018

Conversation

hklai
Copy link
Contributor

@hklai hklai commented Feb 22, 2018

No description provided.

@hklai hklai requested a review from a team February 22, 2018 20:23
@istio-merge-robot
Copy link

@hklai PR needs rebase

@istio-merge-robot istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 22, 2018
yutongz
yutongz approved these changes Feb 22, 2018
View reviewed changes
Copy link
Contributor

@yutongz yutongz left a comment

Choose a reason for hiding this comment

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

/lgtm

mattdelco
mattdelco reviewed Feb 22, 2018
View reviewed changes
release/pipeline/configs/environment_config.py
@@ -26,7 +26,7 @@
MFEST_FILE='build.xml',
MFEST_COMMIT='master@{{{timestamp}}}',
VERSION='{major}.{minor}.{patch}-pre{date}-{rc}',
GCS_BUILD_BUCKET='istio-release-pipline-data',
GCS_BUILD_BUCKET='istio-release-pipeline-data',
Copy link
Contributor

Choose a reason for hiding this comment

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

Be sure to create the bucket.

Copy link
Contributor

Choose a reason for hiding this comment

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

Line 46 also has an instance of "pipline", but it's too far away from any other change for github to allow me to comment on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I have already created the bucket.

Also updated line 46. Failed to take care of the capital P when I grepped.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hrm, the bucket didn't exist when I made the comment. Anyway, it does seem to exist now. I'm not sure if the location matters, but for now I'm doing to assume that multi-regional is a strict superset of a specific region (which is what the original used).

Copy link
Contributor

Choose a reason for hiding this comment

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

in which project was the bucket created ?

Copy link
Contributor

Choose a reason for hiding this comment

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

istio-release. This is also the project used for running the daily builds

@istio-merge-robot
Copy link

/lgtm cancel //PR changed after LGTM, removing LGTM. @hklai @yutongz

@yutongz
Copy link
Contributor

yutongz commented Feb 23, 2018

/lgtm

@istio-merge-robot istio-merge-robot removed the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 23, 2018
@istio-merge-robot
Copy link

@hklai PR needs rebase

@istio-merge-robot istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 23, 2018
@hklai hklai closed this Feb 26, 2018
@hklai
Copy link
Contributor Author

hklai commented Feb 26, 2018

/open

@hklai hklai reopened this Feb 26, 2018
@istio-merge-robot
Copy link

/lgtm cancel //PR changed after LGTM, removing LGTM. @hklai @yutongz

@istio-merge-robot istio-merge-robot removed lgtm needs-rebase Indicates a PR needs to be rebased before being merged labels Feb 26, 2018
@yutongz
Copy link
Contributor

yutongz commented Feb 26, 2018

/lgtm

@hklai
Copy link
Contributor Author

hklai commented Feb 26, 2018

/approve

@rkpagadala
Copy link
Contributor

/lgtm

@istio-merge-robot
Copy link

/lgtm cancel //PR changed after LGTM, removing LGTM. @hklai @rkpagadala @yutongz

@yutongz
Copy link
Contributor

yutongz commented Feb 26, 2018

/lgtm

@mattdelco
Copy link
Contributor

/lgtm
though you can also kill a second bird with this stone by fixing the 4 instances of "quilification" in pipline/dags/istio_common_dag.py.

@codecov
Copy link

codecov bot commented Feb 26, 2018 •
edited
Loading

Codecov Report

Merging #3698 into master will increase coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #3698    +/-   ##
=======================================
+ Coverage      76%     77%    +1%     
=======================================
  Files         291     291            
  Lines       26952   26374   -578     
=======================================
- Hits        20461   20100   -361     
+ Misses       5183    5019   -164     
+ Partials     1308    1255    -53
Impacted Files Coverage Δ
mixer/adapter/solarwinds/log_handler.go 58% <0%> (-11%) ⬇️
mixer/adapter/list/regexList.go 69% <0%> (-9%) ⬇️
security/pkg/platform/gcp.go 95% <0%> (-5%) ⬇️
mixer/adapter/stackdriver/metric/distribution.go 83% <0%> (-4%) ⬇️
mixer/adapter/prometheus/prometheus.go 80% <0%> (-3%) ⬇️
mixer/adapter/redisquota/redisquota.go 86% <0%> (-3%) ⬇️
mixer/adapter/servicecontrol/servicecontrol.go 59% <0%> (-3%) ⬇️
...olarwinds/internal/papertrail/papertrail_logger.go 59% <0%> (-3%) ⬇️
mixer/tools/adapterlinter/main.go 81% <0%> (-1%) ⬇️
mixer/adapter/list/ipList.go 100% <0%> (ø) ⬆️
... and 8 more

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 946710b...5786a23. Read the comment docs.

@istio-merge-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link

/lgtm cancel //PR changed after LGTM, removing LGTM. @hklai @mattdelco @rkpagadala @yutongz

@mattdelco
Copy link
Contributor

/lgtm

@rkpagadala
Copy link
Contributor

/lgtm

@istio-merge-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hklai, mattdelco, rkpagadala, yutongz

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link

Automatic merge from submit-queue.

@istio-merge-robot istio-merge-robot merged commit 893fe73 into master Feb 27, 2018
@ldemailly ldemailly deleted the hklai-airflow branch March 8, 2018 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
3 more reviewers

@sebastienvas sebastienvas sebastienvas left review comments

@mattdelco mattdelco mattdelco left review comments

@yutongz yutongz yutongz approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@rkpagadala rkpagadala

@yutongz yutongz

@mattdelco mattdelco

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@hklai @istio-merge-robot @yutongz @rkpagadala @mattdelco @sebastienvas @googlebot @istio-testing