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

File prompt quiet changes to improve Cisco IOS performance #913

Merged
merged 12 commits into from
Jan 25, 2019

Conversation

ktbyers
Copy link
Contributor

@ktbyers ktbyers commented Jan 17, 2019

No description provided.

@coveralls
Copy link

coveralls commented Jan 17, 2019

Coverage Status

Coverage decreased (-0.04%) to 80.246% when pulling d009ea6 on file_prompt_quiet into 51532c4 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 80.227% when pulling 5f09fe8 on file_prompt_quiet into bd9ea11 on develop.

@vladola
Copy link

vladola commented Jan 18, 2019

I see you went from 7f41fbb to b4f1e4f (from 1 variable to 2 variables).

I did think it could be done with 1 variable at the beginning but let go of the idea soon after, as you did apparently 👍

I wonder if writing the reasoning behind the code might come in handy next time to avoid wasting your time. My variable naming is also often questionable to other people, though (as biased as I am) I still think a less pretty but more descriptive variable name is more important that a more concise yet unclear one.

Anyway, thanks for working on this! This is a classic low-hanging fruit that has massive impact.

@ktbyers
Copy link
Contributor Author

ktbyers commented Jan 18, 2019

@vladola Yes, this solution ended up pretty similar to what you did.

I need to check it some more as I was working on it in a pretty disjointed way yesterday (I was traveling).

@vladola
Copy link

vladola commented Jan 19, 2019

I think you did it! Commit 0a2e7ff still uses 2 variables but I think that's not necessary anymore with your new arrangement. You use self.prompt_quiet_changed in the close method but you can easily just reference self.prompt_quiet_configured and delete self.prompt_quiet_changed altogether.

Neat 👍

@ktbyers ktbyers merged commit 7197e93 into develop Jan 25, 2019
ExaneServerTeam pushed a commit to ExaneServerTeam/napalm that referenced this pull request Mar 6, 2020
@mirceaulinic mirceaulinic deleted the file_prompt_quiet branch May 6, 2020 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants