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

[PR #199/4130c914 backport][stable-2] Small edgeos_config improvements #202

Conversation

patchback[bot]
Copy link

@patchback patchback bot commented Feb 4, 2021

This is a backport of PR #199 as merged into main (4130c91).

SUMMARY
  • The module was littered with magic strings ("set" and "delete") which were too broad a match (because to be commands, they have to be followed by a space).
  • The logical flow to handle when commands started with neither "set" nor "delete" is stated more easily using an "else" block, avoiding duplication of the .startswith() checks
  • Using regular expressions to replace plain strings is overkill
ISSUE TYPE
  • Bugfix Pull Request

(not really, just cleanup)

COMPONENT NAME

edgeos_config

ADDITIONAL INFORMATION

* edgeos_config: Match space after set/delete commands

This makes matching the commands a bit more robust and also removes
magic strings from the code by centralizing them in variables.

* edgeos_config: Use simple string replacements instead of re

* edgeos_config: Simplify logical flow

We do not need to check for each .startswith twice, we can simply use an
else: block

* Update changelogs/fragments/199-edgeos-improvements.yaml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 4130c91)
@dericcrago dericcrago merged commit f1566ff into stable-2 Feb 4, 2021
@dericcrago dericcrago deleted the patchback/backports/stable-2/4130c914c4bf7d449f010c8bd23e5573713a3423/pr-199 branch February 4, 2021 20:14
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.

2 participants