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

Add clearer guidelines on modifying changelog #2739

Merged
merged 4 commits into from
Apr 5, 2017

Conversation

derekbekoe
Copy link
Member

This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • The PR has modified HISTORY.rst with an appropriate description of the change.

Command Guidelines

  • Each command and parameter has a meaningful description.
  • Each new command has a test.

(see Authoring Command Modules)

@codecov-io
Copy link

codecov-io commented Apr 3, 2017

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2739      +/-   ##
==========================================
+ Coverage   61.13%   62.82%   +1.68%     
==========================================
  Files         480      480              
  Lines       25700    25784      +84     
  Branches     3888     3905      +17     
==========================================
+ Hits        15712    16199     +487     
+ Misses       9058     8576     -482     
- Partials      930     1009      +79
Impacted Files Coverage Δ
...twork/azure/cli/command_modules/network/_format.py 40.27% <0%> (-4.73%) ⬇️
...ource/azure/cli/command_modules/resource/custom.py 51.14% <0%> (-3.57%) ⬇️
...e-cli-vm/azure/cli/command_modules/vm/_vm_utils.py 76% <0%> (-1.78%) ⬇️
...zure-cli-vm/azure/cli/command_modules/vm/custom.py 72.57% <0%> (-0.4%) ⬇️
...-cli-role/azure/cli/command_modules/role/custom.py 19.28% <0%> (-0.2%) ⬇️
...dback/azure/cli/command_modules/feedback/custom.py 34.69% <0%> (ø) ⬆️
...i-cloud/azure/cli/command_modules/cloud/_params.py 92.3% <0%> (ø) ⬆️
...ure-cli-dls/azure/cli/command_modules/dls/_help.py 100% <0%> (ø) ⬆️
src/azure-cli-core/azure/cli/core/_profile.py 84.36% <0%> (ø) ⬆️
...-cli-lab/azure/cli/command_modules/lab/commands.py 100% <0%> (ø) ⬆️
... and 51 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 ce75078...507e66d. Read the comment docs.

Answer here:

**OS Version:** What OS and version are you using?
Answer here:

**Shell Type:** What shell are you using? (e.g. bash, cmd.exe, PowerShell)
**Shell Type:** What shell are you using? (e.g. bash, cmd.exe, PowerShell, Bash on Windows)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should put Powershell in this list since it's not one we support. This makes it look like we do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed PowerShell.

Answer here:

**OS Version:** What OS and version are you using?
Answer here:

**Shell Type:** What shell are you using? (e.g. bash, cmd.exe, PowerShell)
**Shell Type:** What shell are you using? (e.g. bash, cmd.exe, PowerShell, Bash on Windows)
Answer here:

### Description
Copy link
Member

Choose a reason for hiding this comment

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

Since you put the separator at the top... shouldn't the description be up there? If not, then the horizontal separator should be before description...

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the separator to be above the description.

### Environment summary

**Install Method:** How did you install the CLI? (e.g. pip, interactive script, apt-get, Docker, MSI, nightly)
Answer here:

**CLI Version:** What version of the CLI and modules are installed? (Available with `az --version`)
**CLI Version:** What version of the CLI and modules are installed? (The full output of `az --version`)
Copy link
Member

@tjprescott tjprescott Apr 4, 2017

Choose a reason for hiding this comment

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

I think what you are going for here is "Put the version of the MODULE you are using, not just azure-cli." Maybe there's a way you could word it to encourage that without resulting in people always posting every module version (which is and will continue to be annoying since 95% of that is always irrelevant). Up to you.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've slightly changed the wording here.
Would still prefer more info that less when debugging.

@derekbekoe
Copy link
Member Author

@tjprescott Feedback addressed, please take a look.

@tjprescott tjprescott merged commit 8dda58c into Azure:master Apr 5, 2017
@derekbekoe derekbekoe deleted the histories branch April 5, 2017 20:54
johanste pushed a commit to johanste/azure-cli that referenced this pull request Apr 7, 2017
* Add clearer guidelines on modifying changelog

* A few smaller changes

* another small format change

* Code review changes
tjprescott pushed a commit that referenced this pull request Apr 11, 2017
* Enable delay-load of descriptions for commands (speed up az)

* Update find indexing commands to accept callables for description.

* Command load time in progress

* - Moved previously dead command filter from parser to application configuration.
- Removed unused configuration object/argv on application create.

* Remove unused argument (pylint)

* Remove dummy parameter

* Fix for python 2.7

* Fix yet incorrect passage of parameters

* Fix up additional pylint complaints

* Update tests

* Update tests

* Fix up more tests

* Fix up more core tests

* Enable delay-load of descriptions for commands (speed up az)

* Update find indexing commands to accept callables for description.

* [Network] Remove nulls from VPN connection show/list output (#2748)

* Fix #1615.

* Code review feedback.

* Update test docs for running individual test and all tests in mod (#2763)

* Update test docs for running individual test and all tests in mod

* Made feedback changes

* Make argument parameters match up. (#2717)

Make lock command parameter aliases match up with resource commands.

* [DevTestLabs] Adding scenario test to create simple Linux + Windows VM in lab (#2767)

* WIP create linux + Windows vm in lab

* Adding recording

* Add some more error checking/handling. (#2768)

Add more validation to resolve "lock level" for lock commands.

* Fix doc references to azure.cli.commands (#2740)

* Fix doc references to azure.cli.commands

This module has moved to azure.cli.core.commands

* Fix PyLint

* Add clearer guidelines on modifying changelog (#2739)

* Add clearer guidelines on modifying changelog

* A few smaller changes

* another small format change

* Code review changes

* [DevTestLabs] Exposing commands to manage secrets in the lab (#2691)

* ACS Update: nulling out the windows profile so that there isn't a validation fail… (#2764)

* nulling out the windows profile so that there isn't a valdiation failure for missing password

ACS doesn't return a password on GET. az acs scale command does a GET
then PUT, but since ACS doesn't return the password the verification is
failing before the PUT is sent to ACS.

There is a bug in ACS this exposes. So this shouldn't be merged until
after the ACS rollout finishes. Should be about start of next week.

* updating history

* updating version in history

* removing white space added by editor

* [Compute] Fix issues with VMSS and VM availability set update. (#2773)

* Fix issues with VMSS and VM availability set update.

* Update help. Fix #2762.

* Error out if you try to list resources for a group that doesn't exist. (#2769)

* Minor text fixes (#2776)

* Add docs for az lock update. (#2702)

* [DevTestLab] Explicitly enable usage of saved secrets while lab vm creation (#2686)

* Explicitly enable usage of saved secrets for vm creation

* Better error message with not overriding competing paramters

* Adding export-artifacts commands on formula (#2707)

* core: apply configured defaults on optional arg (#2770)

* Core:apply configured defaults on optional argument

* add a test

* add tests

* update history doc

* address review feedback

* [Network] Support active-active VNet gateways (#2751)

* Start active-active test scenario.

* Add active-active parameter.

* Active-active scenario test 1 (cross premise)

* Add second active-active scenario (vnet-to-vnet)

* Refine active-active gateway configuration.

* Pylint...

* Code review feedback

* Packaged release notes and changes for 0.2.4 (#2735)

* Modify HISTORY.md

* Update Dockerfile

* Update debian also

* Add pip dependencies also

* Command load time in progress

* - Moved previously dead command filter from parser to application configuration.
- Removed unused configuration object/argv on application create.

* Remove unused argument (pylint)

* Remove dummy parameter

* Fix for python 2.7

* Fix yet incorrect passage of parameters

* Fix up additional pylint complaints

* Update tests

* Update tests

* Fix up more tests

* Fix up more core tests

* Improve load time of custom.py for profile, find and configure (speeds up raw az command)

* Pylint + flake8 fixes

* Fix new vm tests that failed due to perf refactoring

* Update redis tests that was broken due to perf refactoring

* Delay-load msrest for command executions that don't need it

* Fix flake8 issues

* Fixing/improving detection of pageable class

* flake8 fixes

* Fix broken merge from upstream/master

* Fix broken merge (again)

* flake8 fixes

* Fix up even more merge errors from last upstream merge

* Flake8 fixes (wrong number of newlines)

* Fix delay load of storage assembly for az

* Update history to reference improved performance
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.

None yet

5 participants