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

[minor_change] add support for configuration of system fabric wide se… #504

Merged
merged 1 commit into from
Oct 31, 2023
Merged

[minor_change] add support for configuration of system fabric wide se… #504

merged 1 commit into from
Oct 31, 2023

Conversation

akinross
Copy link
Collaborator

…ttings with aci_fabric_wide_settings module

This is a single module (aci_fabric_wide_settings) from a larger PR of tim cragg #419

Splitting up the module for easier review

@akinross akinross added the enhancement New feature or request label Oct 23, 2023
@akinross akinross self-assigned this Oct 23, 2023
samiib
samiib previously approved these changes Oct 24, 2023
View reviewed changes
Copy link
Collaborator

@samiib samiib left a comment

Choose a reason for hiding this comment

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

LGTM 👍

shrsr
shrsr previously approved these changes Oct 24, 2023
View reviewed changes
Copy link
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

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

LGTM

@akinross akinross dismissed stale reviews from shrsr and samiib via f8d71a1 October 25, 2023 14:14
@akinross akinross force-pushed the tim_cragg_fabric_wide_settings branch from dfe9d91 to f8d71a1 Compare October 25, 2023 14:14
@codecov
Copy link

codecov bot commented Oct 25, 2023 •
edited
Loading

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (a82f929) 35.57% compared to head (9d5b45a) 35.61%.

❗ Current head 9d5b45a differs from pull request most recent head a89003c. Consider uploading reports for the commit a89003c to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #504      +/-   ##
==========================================
+ Coverage   35.57%   35.61%   +0.03%     
==========================================
  Files         191      192       +1     
  Lines        8782     8823      +41     
  Branches     1303     1307       +4     
==========================================
+ Hits         3124     3142      +18     
- Misses       5658     5681      +23
Flag Coverage Δ
sanity 35.61% <43.90%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
plugins/module_utils/constants.py 100.00% <100.00%> (ø)
plugins/modules/aci_fabric_wide_settings.py 42.50% <42.50%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shrsr shrsr requested review from shrsr and samiib October 25, 2023 17:32
shrsr
shrsr previously approved these changes Oct 25, 2023
View reviewed changes
Copy link
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

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

LGTM

gmicol
gmicol previously approved these changes Oct 31, 2023
View reviewed changes
Copy link
Collaborator

@gmicol gmicol left a comment

Choose a reason for hiding this comment

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

LGTM

@lhercot
Copy link
Member

lhercot commented Oct 31, 2023

Need rebase

lhercot
lhercot previously approved these changes Oct 31, 2023
View reviewed changes
Copy link
Member

@lhercot lhercot left a comment

Choose a reason for hiding this comment

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

LGTM

@lhercot lhercot dismissed stale reviews from gmicol, shrsr, and themself via a89003c October 31, 2023 22:17
@lhercot lhercot force-pushed the tim_cragg_fabric_wide_settings branch from f8d71a1 to a89003c Compare October 31, 2023 22:17
@lhercot lhercot merged commit 1474956 into CiscoDevNet:master Oct 31, 2023
17 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmicol gmicol gmicol left review comments

@shrsr shrsr shrsr left review comments

@lhercot lhercot lhercot left review comments

@anvitha-jain anvitha-jain Awaiting requested review from anvitha-jain anvitha-jain is a code owner

@sajagana sajagana Awaiting requested review from sajagana sajagana is a code owner

@samiib samiib Awaiting requested review from samiib samiib is a code owner

Assignees

@akinross akinross

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@akinross @lhercot @shrsr @samiib @gmicol