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

[Juniper] Migrating to SONiC Platform API package #8074

Closed
wants to merge 1 commit into from
Closed

[Juniper] Migrating to SONiC Platform API package #8074

wants to merge 1 commit into from

Conversation

ciju-juniper
Copy link
Contributor

This patch set introduce platform API package for QFX5210 & QFX5200 platforms

Signed-off-by: Ciju Rajan K crajank@juniper.net

@ciju-juniper
Copy link
Contributor Author

Test logs on both the platforms are attached:

QFX5210_commands_log.txt

QFX5200_command_log.txt

@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2021

This pull request introduces 57 alerts and fixes 4 when merging 8698f9b2838165389b5578969695125369f19d78 into f14430b - view on LGTM.com

new alerts:

  • 25 for Unused import
  • 17 for Wrong number of arguments in a class instantiation
  • 6 for Unused local variable
  • 3 for Unreachable code
  • 2 for Module imports itself
  • 2 for 'import *' may pollute namespace
  • 2 for Variable defined multiple times

fixed alerts:

  • 4 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2021

This pull request introduces 27 alerts and fixes 4 when merging 2d12b00023abd2f88cd4502b9ddda6fd55971f69 into 0c9862d - view on LGTM.com

new alerts:

  • 17 for Wrong number of arguments in a class instantiation
  • 2 for Module imports itself
  • 2 for Unused local variable
  • 2 for Unreachable code
  • 2 for 'import *' may pollute namespace
  • 1 for Unused import
  • 1 for Variable defined multiple times

fixed alerts:

  • 4 for Unused import

@ciju-juniper
Copy link
Contributor Author

@lguohan @jleveque Could you consider merging this patch set?

@ciju-juniper
Copy link
Contributor Author

This PR fixes the issue #5986

@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2021

This pull request introduces 27 alerts and fixes 4 when merging 40320e262a7ac1bff0977c2e18b2636c4d2e27f8 into 0c9862d - view on LGTM.com

new alerts:

  • 17 for Wrong number of arguments in a class instantiation
  • 2 for Module imports itself
  • 2 for Unused local variable
  • 2 for Unreachable code
  • 2 for 'import *' may pollute namespace
  • 1 for Unused import
  • 1 for Variable defined multiple times

fixed alerts:

  • 4 for Unused import

@ciju-juniper
Copy link
Contributor Author

@lguohan @jleveque Could you review and approve the patches? This patch set will make Juniper platforms functional on master branch which was broken for a while.

@ciju-juniper
Copy link
Contributor Author

@lguohan @jleveque Could you please take a look?

@lguohan
Copy link
Collaborator

lguohan commented Jul 14, 2021

can you fix lgtm alerts?

@ciju-juniper
Copy link
Contributor Author

@lguohan Most of the alerts are due to this warning Wrong number of arguments to constructor initialization from a for loop.

Eg:
thermal = Thermal(idx)

For which constructor is having the initialization routine
def init(self, thermal_index=0):

I'm not sure what is wrong with the call / definitions. I've already suppressed it using '#lgtm [py/call/wrong-number-class-arguments]',

@ciju-juniper
Copy link
Contributor Author

@lguohan All the false positive warnings are marked with the comment '#lgtm [...]'. You could see it by opening the lgtm link: https://lgtm.com/projects/g/Azure/sonic-buildimage/rev/pr-f582fa5952ed702151a827f96acf2f5d1bf5f363

Lgtm says, the warnings will go away only when the pull request is merged, if the updates are made from a pull request.

Please merge the patch set.

@ciju-juniper
Copy link
Contributor Author

@lguohan Would you be able to merge this patch set as there is no way as per the LGTM to show the warnings as zero in the pull request?

This patch set introduce platform API package for
QFX5210 & QFX5200 platforms

Signed-off-by: Ciju Rajan K <crajank@juniper.net>
@lgtm-com
Copy link

lgtm-com bot commented Jul 26, 2021

This pull request introduces 27 alerts and fixes 4 when merging df935a3 into 5e435e0 - view on LGTM.com

new alerts:

  • 17 for Wrong number of arguments in a class instantiation
  • 2 for Module imports itself
  • 2 for Unused local variable
  • 2 for Unreachable code
  • 2 for 'import *' may pollute namespace
  • 1 for Unused import
  • 1 for Variable defined multiple times

fixed alerts:

  • 4 for Unused import

@ciju-juniper
Copy link
Contributor Author

Closing this pull request as lgtm is not able to show the fixed alerts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants