Skip to content

DellEMC: S5248F - Platform API 2.0 implementation#7930

Merged
jleveque merged 7 commits intosonic-net:masterfrom
arunlk-dell:api2.0
Jun 28, 2021
Merged

DellEMC: S5248F - Platform API 2.0 implementation#7930
jleveque merged 7 commits intosonic-net:masterfrom
arunlk-dell:api2.0

Conversation

@arunlk-dell
Copy link
Copy Markdown
Contributor

@arunlk-dell arunlk-dell commented Jun 21, 2021

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
Copy Markdown

ghost commented Jun 21, 2021 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@lgtm-com
Copy link
Copy Markdown

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

jleveque
jleveque suggested changes Jun 21, 2021
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
Contributor Author

arunlk-dell commented Jun 22, 2021

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

@jleveque
Copy link
Copy Markdown
Contributor

jleveque commented Jun 22, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Jun 22, 2021

Azure Pipelines successfully started running 1 pipeline(s).

jleveque
jleveque suggested changes Jun 22, 2021
View reviewed changes
@lgtm-com
Copy link
Copy Markdown

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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
Contributor Author

arunlk-dell commented Jun 25, 2021

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

@jleveque
Copy link
Copy Markdown
Contributor

jleveque commented Jun 25, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Jun 25, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@arunlk-dell
Copy link
Copy Markdown
Contributor Author

arunlk-dell commented Jun 28, 2021

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
@arunlk-dell @qiluo-msft
DellEMC: S5248F - Platform API 2.0 implementation (#7930)
a1268b3 
#### 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>
@arunlk-dell arunlk-dell mentioned this pull request Jul 5, 2021
Open
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
@arunlk-dell
DellEMC: S5248F - Platform API 2.0 implementation (sonic-net#7930)
a9594f0 
#### 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

Reviewers

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

1 more reviewer

@jleveque jleveque jleveque approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Enhancement ➕ Included in 202012 Branch Included in 202106 Branch Platform: DellEMC Request for 202012 Branch Request for 202106 Branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@arunlk-dell @jleveque @qiluo-msft @judyjoseph