Skip to content

[DellEMC Z9332f] Platform API 2.0 Support and bug fixing#5958

Merged
jleveque merged 7 commits intosonic-net:masterfrom
srideepDell:z9332_platform_2.0_api_support
Dec 10, 2020
Merged

[DellEMC Z9332f] Platform API 2.0 Support and bug fixing#5958
jleveque merged 7 commits intosonic-net:masterfrom
srideepDell:z9332_platform_2.0_api_support

Conversation

@srideepDell
Copy link
Copy Markdown
Contributor

       1) Add platform infra to support 2.0 API
       2) Bug fixing for 9332 known issues

- Why I did it
Add platform infra to support 2.0 API
Bug fixing for 9332 known issues

- How I did it
Change to platform specific code under platform directory specific to Z9332

- How to verify it
verified on Z9332 platform logs attached

x

- Description for the changelog
1) Add platform infra to support 2.0 API
2) Bug fixing for 9332 known issues

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

           1) Add platform infra to support 2.0 APi
           2) Bug fixing for 9332 known issues
@srideepDell
Copy link
Copy Markdown
Contributor Author

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Nov 18, 2020

This pull request introduces 34 alerts when merging 4bc16a8 into 2fe79c2 - view on LGTM.com

new alerts:

  • 10 for Unused import
  • 6 for Except block handles 'BaseException'
  • 6 for Wrong number of arguments in a class instantiation
  • 4 for Variable defined multiple times
  • 2 for Unused local variable
  • 1 for Unnecessary pass
  • 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

@srideepDell srideepDell marked this pull request as ready for review November 18, 2020 19:05
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Nov 18, 2020

This pull request introduces 23 alerts when merging 798b608 into bbbd94f - view on LGTM.com

new alerts:

  • 6 for Except block handles 'BaseException'
  • 6 for Wrong number of arguments in a class instantiation
  • 3 for Variable defined multiple times
  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for Unnecessary pass
  • 1 for Module imports itself
  • 1 for 'import *' may pollute namespace
  • 1 for Wrong name for an argument in a class instantiation

@srideepDell
Copy link
Copy Markdown
Contributor Author

All the critical and relevant lgtm issues are fixed.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Dec 2, 2020

This pull request introduces 23 alerts when merging 167d56f into 61419f5 - view on LGTM.com

new alerts:

  • 6 for Except block handles 'BaseException'
  • 6 for Wrong number of arguments in a class instantiation
  • 3 for Variable defined multiple times
  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for Unnecessary pass
  • 1 for Module imports itself
  • 1 for 'import *' may pollute namespace
  • 1 for Wrong name for an argument in a class instantiation

@daall daall self-requested a review December 4, 2020 00:15
Copy link
Copy Markdown
Contributor

@daall daall 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 the LGTM warnings!

@jleveque
Copy link
Copy Markdown
Contributor

jleveque commented Dec 4, 2020

Some LGTM alerts may be false alarms (like "Wrong number of arguments in a class instantiation") because LGTM references the incorrect vendor's platform API files. However, some of these can be fixed, like:

  • 6 for Except block handles 'BaseException' (use except Exception: instead of except:)
  • 3 for Variable defined multiple times
  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for Unnecessary pass
  • 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 Dec 7, 2020

This pull request introduces 18 alerts when merging 0b026e6 into 3b04da9 - view on LGTM.com

new alerts:

  • 6 for Wrong number of arguments in a class instantiation
  • 3 for Variable defined multiple times
  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for Unnecessary pass
  • 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
Copy Markdown

lgtm-com bot commented Dec 8, 2020

This pull request introduces 9 alerts when merging f59307f into 43a32e6 - view on LGTM.com

new alerts:

  • 6 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 Dec 8, 2020

This pull request introduces 9 alerts when merging c5270c2 into 275c5cf - view on LGTM.com

new alerts:

  • 6 for Wrong number of arguments in a class instantiation
  • 2 for Variable defined multiple times
  • 1 for Wrong name for an argument in a class instantiation

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Dec 9, 2020

This pull request introduces 7 alerts when merging 3bb0121 into 275c5cf - view on LGTM.com

new alerts:

  • 6 for Wrong number of arguments in a class instantiation
  • 1 for Wrong name for an argument in a class instantiation

@jleveque
Copy link
Copy Markdown
Contributor

jleveque commented Dec 9, 2020

@srideepDell: Thanks for working to clean up those LGTM alerts!

@daall
Copy link
Copy Markdown
Contributor

daall commented Dec 9, 2020

retest vs please

@srideepDell
Copy link
Copy Markdown
Contributor Author

@srideepDell: Thanks for working to clean up those LGTM alerts!

Thanks for your guidance Joe

@srideepDell
Copy link
Copy Markdown
Contributor Author

retest vs please

@srideepDell
Copy link
Copy Markdown
Contributor Author

@jleveque @daall All checks have passed. Could you help merge the PR.

@jleveque jleveque merged commit 3c9a7ec into sonic-net:master Dec 10, 2020
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.

3 participants