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

Renamed chef-service-interval to chef-daemon-interval #3799

Merged
merged 3 commits into from
May 2, 2017

Conversation

piyushawasthi
Copy link
Contributor

@piyushawasthi piyushawasthi commented Apr 19, 2017

  • Renamed chef-service-interval to chef-daemon-interval.
  • Fixed failing tests.

@msftclas
Copy link

@piyushawasthi,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@azuresdkci
Copy link

Can one of the admins verify this patch?

@markcowl markcowl changed the base branch from dev to preview April 19, 2017 16:58
@markcowl
Copy link
Member

@piyushawasthi you will need to sign the CLA, or, if you are internal, follow the process here: http://aka.ms/azuregithub before we can look at this PR

@markcowl markcowl removed their assignment Apr 19, 2017
@msftclas
Copy link

@piyushawasthi, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@piyushawasthi
Copy link
Contributor Author

@markcowl - I have signed the CLA, Thanks a lot for your help.

@markcowl
Copy link
Member

@azuresdkci add to whitelist

@twitchax @singhkays We ened a reviewer for this VM extention

@singhkays
Copy link

This cmdlet is written by the Chef team. Adding @NimishaS for review.

@NimishaS
Copy link

Testing the PR.

@cormacpayne
Copy link
Member

@piyushawasthi would you mind pulling the latest changes from the preview branch? These changes removed the dll-Help.xml file from the repository, so you will no longer need to update this file in your PR.

@piyushawasthi piyushawasthi force-pushed the piyush/chef_daemon_interval branch from 2cad248 to e499056 Compare April 26, 2017 12:48
@piyushawasthi
Copy link
Contributor Author

@cormacpayne - Re-base branch from updated "preview" branch and resolved the conflict. Also verified using ARM and ASM command -ChefDaemonInterval getting set properly.

@@ -78,7 +78,7 @@ public class SetAzureVMChefExtensionCommand : VirtualMachineChefExtensionCmdletB
ValueFromPipelineByPropertyName = true,
HelpMessage = "Specifies the frequency (in minutes) at which the chef-service runs. If in case you don't want the chef-service to be installed on the Azure VM then set value as 0 in this field.")]
[ValidateNotNullOrEmpty]
public string ChefServiceInterval { get; set; }
public string ChefDaemonInterval { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

Can you add an alias here so that old scripts will not break?

@@ -30873,7 +30873,7 @@
"ValidateNotNullOrEmpty": true
Copy link
Member

Choose a reason for hiding this comment

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

If you add the alias, there would be no breaking change here. If it turns out you do need a breaking change, please add a suppression here: https://github.com/Azure/azure-powershell/blob/preview/tools/StaticAnalysis/Exceptions/BreakingChangeIssues.csv

Rather than updating this json file.

Copy link
Member

Choose a reason for hiding this comment

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

Please remove this change, and please add a description of the change (the new parameter) to the Changelog.md for your module, here: https://github.com/Azure/azure-powershell/blob/preview/src/ResourceManager/Compute/ChangeLog.md

@markcowl
Copy link
Member

@piyushawasthi A couple of changes, otherwise LGTM

@piyushawasthi
Copy link
Contributor Author

piyushawasthi commented Apr 28, 2017

@markcowl : I have added alias here: 5dc4ca8

Please let me know if still required to add new entry in BreakingChangeIssues.csv file.

Copy link
Member

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

Looks good. Please remove the change to the json file, as specified, and please add a comment on the change to the chngelog here: https://github.com/Azure/azure-powershell/blob/preview/src/ResourceManager/Compute/ChangeLog.md as specified.

@@ -30873,7 +30873,7 @@
"ValidateNotNullOrEmpty": true
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this change, and please add a description of the change (the new parameter) to the Changelog.md for your module, here: https://github.com/Azure/azure-powershell/blob/preview/src/ResourceManager/Compute/ChangeLog.md

@markcowl
Copy link
Member

@piyushawasthi Please remove the json changes and add changelog summary for changes and this will be ready to merge

@piyushawasthi piyushawasthi force-pushed the piyush/chef_daemon_interval branch 2 times, most recently from 09d3449 to 7b89303 Compare May 2, 2017 05:01
@piyushawasthi piyushawasthi force-pushed the piyush/chef_daemon_interval branch from 7b89303 to 910a2b7 Compare May 2, 2017 05:22
@piyushawasthi
Copy link
Contributor Author

piyushawasthi commented May 2, 2017

@markcowl : Update changes as per your comment.

@markcowl
Copy link
Member

markcowl commented May 2, 2017

@cormacpayne cormacpayne merged commit 066671e into Azure:preview May 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants