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

DellEMC: S5248F - Platform API 2.0 implementation #7930

Merged
merged 7 commits into from
Jun 28, 2021

Conversation

arunlk-dell
Copy link
Contributor

Why I did it

Support API 2.0 for S5248F platform

How I did it

Making changes to S5248F platform specific directory

How to verify it

With a python script verified the platform API return values.
Verified 'show platform' and 'show interface transceiver' related command outputs.
s5248f_UnitTest logs.txt

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

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

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

@ghost
Copy link

ghost commented Jun 21, 2021

CLA assistant check
All CLA requirements met.

@lgtm-com
Copy link

lgtm-com bot commented Jun 21, 2021

This pull request introduces 20 alerts when merging 666dcca into 9d1c165 - view on LGTM.com

new alerts:

  • 7 for Wrong number of arguments in a class instantiation
  • 6 for Unused import
  • 2 for Except block handles 'BaseException'
  • 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

Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

Please fix LGTM alerts

@lgtm-com
Copy link

lgtm-com bot commented Jun 22, 2021

This pull request introduces 11 alerts when merging f995c2a into 5bf083a - view on LGTM.com

new alerts:

  • 7 for Wrong number of arguments in a class instantiation
  • 1 for Module imports itself
  • 1 for Unused import
  • 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 Jun 22, 2021

This pull request introduces 10 alerts when merging cf42020 into 078e0e0 - view on LGTM.com

new alerts:

  • 7 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

@arunlk-dell
Copy link
Contributor Author

Have addressed the LGTM alerts, the wrong name/number of the argument alerts are due to different platform definitions.

@jleveque
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 Jun 24, 2021

This pull request introduces 10 alerts when merging ae02384 into a3894b7 - view on LGTM.com

new alerts:

  • 7 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

@lgtm-com
Copy link

lgtm-com bot commented Jun 24, 2021

This pull request introduces 10 alerts when merging 9ede79a into d6d7cb7 - view on LGTM.com

new alerts:

  • 7 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

@lgtm-com
Copy link

lgtm-com bot commented Jun 24, 2021

This pull request introduces 10 alerts when merging 1948037 into d6d7cb7 - view on LGTM.com

new alerts:

  • 7 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

@arunlk-dell
Copy link
Contributor Author

@joe,
The failure are not due to the changes, can you please initiate the re-run for failure test.

@jleveque
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@arunlk-dell
Copy link
Contributor Author

Have removed the python 2 support, can you review the latest changes and approve for merging.

@jleveque jleveque merged commit 1d9fd82 into sonic-net:master Jun 28, 2021
qiluo-msft pushed a commit that referenced this pull request Jun 29, 2021
#### Why I did it
Support API 2.0 for S5248F platform

#### How I did it
Making changes to S5248F platform specific directory

Co-authored-by: Arun LK <Arun_L_K@dell.com>
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
#### Why I did it
Support API 2.0 for S5248F platform

#### How I did it
Making changes to S5248F platform specific directory

Co-authored-by: Arun LK <Arun_L_K@dell.com>
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