Skip to content

Fix indentation of negotiation commands #118

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

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

Jonher937
Copy link
Contributor

@Jonher937 Jonher937 commented Mar 18, 2022

SUMMARY

Missing prefixed whitespace for a command that should run in a context and not in top-level context.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

os10_interface

ADDITIONAL INFORMATION

Without this change the interface role fails no matter the mode you set autoneg in. Prefixing these commands with a whitespace that should run in a sub-context makes it work.

Fixes problem introduced in #77

@Jonher937
Copy link
Contributor Author

@sc68cal since you pushed #77 can you verify this works on your end as well?
I recently added #108 and updated the role on my staging system, that's when I noticed the interface role is now broken.

@sc68cal
Copy link
Contributor

sc68cal commented Mar 19, 2022

I will test this coming week

@Jonher937
Copy link
Contributor Author

I will test this coming week

Friendly reminder to not forget about this 😄


Also a friendly ping to @Andersson007 since you were helpful last time getting the changes to merge during the absence of the Dell people.
This should probably warrant for a 1.2.1 release in galaxy since the .0 version is broken (at least it is for me).
If we push a 1.2.1 it would be awesome to get #119 included in that as well.

@Andersson007 Andersson007 merged commit 7477449 into ansible-collections:master Mar 24, 2022
@Andersson007
Copy link
Contributor

@Jonher937 thanks for the fix!
@sc68cal thanks for reviewing and approving!

@Andersson007
Copy link
Contributor

@Jonher937 we've pinged Dell folks internally.
i released the collection but it hasn't been published on Galaxy as, i guess, past maintainers did it manually and i don't have an opportunity for that. I hope they'll release 1.2.1 soon and will publish it.

@Jonher937 Jonher937 deleted the fix-negotiation branch March 24, 2022 18:28
@sc68cal
Copy link
Contributor

sc68cal commented Mar 25, 2022

Apologies for the bug. The issue is that most of the Ansible roles that use Jinja 2 templates use two spaces because it's a lot easier to see the mistakes, while this repo only uses one space for indentation. So, my bad.

@Andersson007
Copy link
Contributor

@sc68cal no worries:)

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.

3 participants