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

Added Support for Dell EMC S5212f in SONiC #8678

Merged
merged 9 commits into from
Oct 26, 2021

Conversation

thaj-deen
Copy link
Contributor

Why I did it

Added Support for Dell EMC S5212f platform

How I did it

Implemented the support for Dell EMC S5212f platform

Platform: x86_64-dellemc_s5212f_c3538-r0
HwSKU: DellEMC-S5212f-P-25G
ASIC: broadcom
ASIC Count: 1

How to verify it

Verified the show command outputs

Attached the test logs
S5212-COMM-SONIC-Test-Logs.txt

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

This Pull request contains the Platform and Device dependent files to onboard Dell EMC S5212f

A picture of a cute animal (not mandatory but encouraged)

@thaj-deen thaj-deen requested a review from lguohan as a code owner September 3, 2021 16:22
@ghost
Copy link

ghost commented Sep 3, 2021

CLA assistant check
All CLA requirements met.

@yozhao101
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lgtm-com
Copy link

lgtm-com bot commented Sep 3, 2021

This pull request introduces 44 alerts when merging 6a6b12c into 7c9be31 - view on LGTM.com

new alerts:

  • 16 for Unused import
  • 8 for Wrong number of arguments in a class instantiation
  • 6 for Variable defined multiple times
  • 4 for Unused local variable
  • 3 for Except block handles 'BaseException'
  • 1 for Unnecessary pass
  • 1 for Use of the return value of a procedure
  • 1 for Module imports itself
  • 1 for Unreachable code
  • 1 for 'import *' may pollute namespace
  • 1 for Implicit string concatenation in a list
  • 1 for Wrong name for an argument in a class instantiation

@thaj-deen
Copy link
Contributor Author

Addressed all LGTM alerts except Wrong name/number of the argument alerts are due to different platform definitions.

@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2021

This pull request introduces 30 alerts when merging 8fb6471 into f4dea87 - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 8 for Wrong number of arguments in a class instantiation
  • 4 for Unused local variable
  • 4 for Variable defined multiple times
  • 1 for Unnecessary pass
  • 1 for Use of the return value of a procedure
  • 1 for Module imports itself
  • 1 for Except block handles 'BaseException'
  • 1 for 'import *' may pollute namespace
  • 1 for Wrong name for an argument in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2021

This pull request introduces 14 alerts when merging 93b0fa7 into f4dea87 - view on LGTM.com

new alerts:

  • 8 for Wrong number of arguments in a class instantiation
  • 1 for Unnecessary pass
  • 1 for Use of the return value of a procedure
  • 1 for Module imports itself
  • 1 for Except block handles 'BaseException'
  • 1 for 'import *' may pollute namespace
  • 1 for Wrong name for an argument in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2021

This pull request introduces 12 alerts when merging fa790e7 into 63ba489 - view on LGTM.com

new alerts:

  • 8 for Wrong number of arguments in a class instantiation
  • 1 for Use of the return value of a procedure
  • 1 for Module imports itself
  • 1 for 'import *' may pollute namespace
  • 1 for Wrong name for an argument in a class instantiation

@thaj-deen
Copy link
Contributor Author

thaj-deen commented Sep 8, 2021

@sujinmkang @prgeor - Can you review and approve the changes ?
Thanks

sujinmkang
sujinmkang previously approved these changes Sep 13, 2021
Copy link
Collaborator

@sujinmkang sujinmkang left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@thaj-deen can you fix LGTM warnings?

@sujinmkang
Copy link
Collaborator

@thaj-deen Can you rebase this and please address for the prince's comment?

@lgtm-com
Copy link

lgtm-com bot commented Sep 27, 2021

This pull request introduces 12 alerts when merging a0c79ea953073ce08ac9c4d4e1aa837d37f986f8 into c668f2a - view on LGTM.com

new alerts:

  • 8 for Wrong number of arguments in a class instantiation
  • 1 for Use of the return value of a procedure
  • 1 for Module imports itself
  • 1 for 'import *' may pollute namespace
  • 1 for Wrong name for an argument in a class instantiation

@thaj-deen
Copy link
Contributor Author

Merged and resolved the conflicts

@prgeor - Resolved LGTM warning - The wrong name/number of the argument alerts are due to different platform definitions.

@thaj-deen
Copy link
Contributor Author

@thaj-deen Can you rebase this and please address for the prince's comment?

@sujinmkang - Addressed your comment

@thaj-deen thaj-deen closed this Sep 27, 2021
@thaj-deen thaj-deen reopened this Sep 27, 2021
@thaj-deen
Copy link
Contributor Author

@sujinmkang @prgeor - close the PR by mistake

@lgtm-com
Copy link

lgtm-com bot commented Sep 27, 2021

This pull request introduces 11 alerts when merging ec79793 into c668f2a - view on LGTM.com

new alerts:

  • 8 for Wrong number of arguments in a class instantiation
  • 1 for Module imports itself
  • 1 for 'import *' may pollute namespace
  • 1 for Wrong name for an argument in a class instantiation

@sujinmkang sujinmkang requested a review from prgeor October 22, 2021 19:56
@sujinmkang sujinmkang merged commit f01076e into sonic-net:master Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants