Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

refactor: Remove agentpool index usage from function which gets agent VM name #428

Merged
merged 1 commit into from Feb 4, 2019

Conversation

kkmsft
Copy link
Contributor

@kkmsft kkmsft commented Feb 1, 2019

Reason for Change:

Remove the agentpool index usage from upgrade code path.

Issue Fixed:

Remove agentpool index usage from function which gets agent VM name

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Feb 1, 2019

Codecov Report

Merging #428 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #428      +/-   ##
=========================================
- Coverage   53.41%   53.4%   -0.01%     
=========================================
  Files          95      95              
  Lines       14360   14360              
=========================================
- Hits         7670    7669       -1     
- Misses       6027    6028       +1     
  Partials      663     663

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

code lgtm

let's run an upgrade E2E on this since it's changing upgrade code

@CecileRobertMichon
Copy link
Contributor

I'd prefer "refactor" over "fix" for the commit message since this isn't fixing any bugs

@kkmsft
Copy link
Contributor Author

kkmsft commented Feb 1, 2019

@CecileRobertMichon - changed to refactor.

@CecileRobertMichon CecileRobertMichon changed the title fix: Remove agentpool index usage from function which gets agent VM name refactor: Remove agentpool index usage from function which gets agent VM name Feb 1, 2019
@CecileRobertMichon
Copy link
Contributor

lgtm pending E2E and upgrade test

@xizha162
Copy link
Contributor

xizha162 commented Feb 2, 2019

LGTM too! Thanks KK!

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

/lgtm

@acs-bot
Copy link

acs-bot commented Feb 4, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, kkmsft

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

@CecileRobertMichon CecileRobertMichon merged commit 23bfb1e into Azure:master Feb 4, 2019
@welcome
Copy link

welcome bot commented Feb 4, 2019

Congrats on merging your first pull request! 🎉🎉🎉

@kkmsft kkmsft deleted the remove-index-usage branch February 4, 2019 20:28
juhacket pushed a commit to juhacket/aks-engine that referenced this pull request Mar 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants