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 note about being in preview #2512

Merged
merged 11 commits into from Mar 15, 2017
Merged

Add note about being in preview #2512

merged 11 commits into from Mar 15, 2017

Conversation

allclark
Copy link
Contributor

No description provided.

@allclark
Copy link
Contributor Author

@twitchax : This would replace the (Preview) in the titles, which has some problems (for the user, it may imply the service is in preview; for maintenance, this allows the dev team to more easily remove the comment than if it remains in the titlemapping.json file over in the azure-docs-cli-python repo.

@twitchax
Copy link
Contributor

@Azure/azure-cli-team, I think this is a good change, but I cannot quite pinpoint why the tests are failing?

@devigned
Copy link
Member

Why not just put that once at the module level? So when you run az acr -h you can see the module is preview. Perhaps, when you run az -h, each module that is preview shows up with a little preview moniker.

Seems overkill to start splattering the preview phrase on each of the commands.

@allclark
Copy link
Contributor Author

@devigned I agree that the commands should not have the comment. I only applied that comment at the command group level, not the commands. The reason I went down to each command group (not just top level) is that each command group is its own page. If you land on the page for az sql dw, for example, there would be no indication there that SQL is in preview. (We're removing the indication from the titles).

@codecov-io
Copy link

codecov-io commented Mar 15, 2017

Codecov Report

Merging #2512 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2512   +/-   ##
=======================================
  Coverage   72.27%   72.27%           
=======================================
  Files         362      362           
  Lines       19671    19671           
  Branches     2897     2897           
=======================================
  Hits        14217    14217           
  Misses       4539     4539           
  Partials      915      915
Impacted Files Coverage Δ
...ure-cli-iot/azure/cli/command_modules/iot/_help.py 100% <ø> (ø)
...re/cli/command_modules/datalake/analytics/_help.py 100% <ø> (ø)
.../azure/cli/command_modules/datalake/store/_help.py 100% <ø> (ø)
...ainer/azure/cli/command_modules/container/_help.py 100% <ø> (ø)
...ure-cli-acr/azure/cli/command_modules/acr/_help.py 100% <ø> (ø)

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 77a74f6...204db57. Read the comment docs.

@allclark
Copy link
Contributor Author

Looks like I need a review before I can merge.

Copy link
Contributor

@twitchax twitchax left a comment

Choose a reason for hiding this comment

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

My only comment would be to undo the ones that are about to GA (10 days or so). The "preview" flag will need to be removed for them in about a week anyway.

  • App service:
    • Plans
    • Web Apps
    • Deployment Slots
    • Deployment Sources
    • Domains
    • Certificate
  • KeyVault
  • Redis
  • DocDb
  • Batch

@allclark
Copy link
Contributor Author

This is ready to go, but I don't have permission to merge.

@troydai
Copy link
Contributor

troydai commented Mar 15, 2017

I'll merge once the CI passes. Moreover, I assume @twitchax has no problem with the changes committed after his approval.

@troydai
Copy link
Contributor

troydai commented Mar 15, 2017

@allclark one suggestion here. Because our CI will run on every commit in a pull request, it is highly recommended not to submit multiple small commits to tweak the changes too frequently. Otherwise, the minor changes will waste enormous resources while running the automation tests. The Azure organization shares the same pool of Travis CI resources. The recommended solutions are to make changes on your fork and sent a pull request after you finish the changes.

@allclark
Copy link
Contributor Author

@toydai OK Good to know that. Thx

@troydai
Copy link
Contributor

troydai commented Mar 15, 2017

@allclark thank you for your understanding.

@troydai troydai merged commit 41ecccd into Azure:master Mar 15, 2017
@twitchax
Copy link
Contributor

@allclark, don't worry...I am also a frequent perpetrator of this. :P

thegalah pushed a commit to thegalah/azure-cli that referenced this pull request Mar 21, 2017
* Azure/master: (478 commits)
  vm live test: allow more valid power states on vmss test verifications (Azure#2564)
  rbac:catch more graph error (Azure#2567)
  appservice: support to create plan when create a webapp (Azure#2550)
  Update storage tests (Azure#2556)
  Change PEP8 check filter from whitelist to blacklist (Azure#2557)
  Add scenario tests documentation (Azure#2555)
  [ACS] Adding support for configuring a default ACS cluster (Azure#2554)
  [ACS] Provide a short name alias for the orchestrator type flag (Azure#2553)
  Sql Import/Export CLI commands and test (Azure#2538)
  Fix format bug. (Azure#2549)
  [VM/VMSS] Improved disk caching support (Azure#2522)
  VM/VMSS: incorporate credentials validation logic used by portal (Azure#2537)
  Script that creates packaged releases package archive (Azure#2508)
  Adding alias for defaults flag (Azure#2540)
  Add wait commands and --no-wait support (Azure#2524)
  choice list outside of named arguments (Azure#2521)
  Fixed test failure in test_sql_db_mgmt. (Azure#2530)
  core: support login using service principal with a cert (Azure#2457)
  Add note about being in preview (Azure#2512)
  vm:fix distro check mechanism used by disk encryption (Azure#2511)
  ...
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

7 participants