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.

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

Asking the user for feedback using custom heuristic #3139

Merged
merged 18 commits into from
May 5, 2017
Merged

Asking the user for feedback using custom heuristic #3139

merged 18 commits into from
May 5, 2017

Conversation

oakeyc
Copy link
Contributor

@oakeyc oakeyc commented May 2, 2017 •
edited
Loading

fixes #3157

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 (see Modifying change log).

Command Guidelines

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

(see Authoring Command Modules)

@oakeyc
Copy link
Contributor Author

oakeyc commented May 2, 2017

screen shot 2017-05-02 at 2 45 39 pm

@oakeyc oakeyc requested review from derekbekoe and mayurid May 2, 2017 21:48
@codecov-io
Copy link

codecov-io commented May 3, 2017 •
edited
Loading

Codecov Report

Merging #3139 into master will decrease coverage by <.01%.
The diff coverage is 55.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3139      +/-   ##
==========================================
- Coverage   70.34%   70.34%   -0.01%     
==========================================
  Files         383      383              
  Lines       24951    24960       +9     
  Branches     3796     3797       +1     
==========================================
+ Hits        17553    17558       +5     
- Misses       6288     6291       +3     
- Partials     1110     1111       +1
Impacted Files Coverage Δ
...odules/azure-cli-shell/azclishell/configuration.py 60.75% <55.55%> (-0.67%) ⬇️

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 743b4fb...bb78aba. Read the comment docs.

derekbekoe
derekbekoe reviewed May 5, 2017
View reviewed changes
src/command_modules/azure-cli-shell/azclishell/app.py Outdated
try:
args = parse_quotes(cmd)
azlogging.configure_logging(args)

if 'feedback' in args:
Copy link
Member

Choose a reason for hiding this comment

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

What if I create a VM with name 'feedback'.
How does this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

@derekbekoe derekbekoe modified the milestone: Build Milestone May 5, 2017
troydai
troydai reviewed May 5, 2017
View reviewed changes
src/command_modules/azure-cli-shell/azclishell/frequency_heuristic.py Outdated
frequency = {}

with open(os.path.join(shell_config(), SHELL_CONFIG.get_frequency()), 'w') as freq:
now = datetime.datetime.now()
Copy link
Contributor

@troydai troydai May 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

UTC?

troydai
troydai previously approved these changes May 5, 2017
View reviewed changes
Copy link
Contributor

@troydai troydai left a comment

Choose a reason for hiding this comment

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

Better use UTC time.

@troydai
Copy link
Contributor

troydai commented May 5, 2017

Looks good. Please test it again before merge.

@oakeyc oakeyc merged commit 274a9fc into Azure:master May 5, 2017
@oakeyc oakeyc deleted the feedback branch May 5, 2017 19:39
derekbekoe added a commit to derekbekoe/azure-cli that referenced this pull request May 5, 2017
@derekbekoe
Revert "Asking the user for feedback using custom heuristic (Azure#3139…
7914ed8 
…)"

This reverts commit 274a9fc.
derekbekoe added a commit that referenced this pull request May 5, 2017
@derekbekoe
Revert shell feedback heuristics & progress (#3197)
39a8fcd 
* Revert "Asking the user for feedback using custom heuristic (#3139)"

This reverts commit 274a9fc.

* Revert progress change

* minor changes that can be included

* Final fixes
@oakeyc oakeyc restored the feedback branch May 8, 2017 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekbekoe derekbekoe derekbekoe approved these changes

@troydai troydai troydai left review comments

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
Build Milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@oakeyc @codecov-io @troydai @derekbekoe @msftclas @haroldrandom