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.

Sign up for GitHub

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

Jump to bottom

BFN platform API 2.0 support #4766

Merged
merged 9 commits into from
Oct 3, 2020
Merged

BFN platform API 2.0 support #4766

merged 9 commits into from
Oct 3, 2020

Conversation

vboykox
Copy link
Member

@vboykox vboykox commented Jun 12, 2020

- Why I did it
Added barefoot platform api 2.0 support

- Description for the changelog
Added barefoot platform api 2.0 support

@ghost
Copy link

ghost commented Jun 12, 2020 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@akokhan
Copy link
Contributor

akokhan commented Jun 12, 2020

By adding this, I believe you should remove platform 1.0 plugins.

@lgtm-com
Copy link

lgtm-com bot commented Jun 12, 2020

This pull request introduces 28 alerts when merging a296260 into 6acd64d - view on LGTM.com

new alerts:

  • 14 for Unused import
  • 6 for Except block handles 'BaseException'
  • 4 for 'import *' may pollute namespace
  • 2 for Wrong number of arguments in a class instantiation
  • 1 for Unused local variable
  • 1 for Variable defined multiple times

@lgtm-com
Copy link

lgtm-com bot commented Jun 15, 2020

This pull request introduces 18 alerts when merging 7028f3bab51d4c59400e049a363521ea0a2515c5 into 0a750a6 - view on LGTM.com

new alerts:

  • 6 for Except block handles 'BaseException'
  • 4 for Unused import
  • 4 for 'import *' may pollute namespace
  • 2 for Wrong number of arguments in a class instantiation
  • 1 for Unused local variable
  • 1 for Variable defined multiple times

@vboykox
Copy link
Member Author

vboykox commented Jun 17, 2020 •
edited
Loading

By adding this, I believe you should remove platform 1.0 plugins.

decode-eeprom, sfputil, psuutil still use API 1.0

The thread on sonicproject mailing list, please join the discussion:
https://groups.google.com/d/msgid/sonicproject/09d8fec2-480e-4138-b91f-8946eeebed32o%40googlegroups.com?utm_medium=email&utm_source=footer

@vboykox
Copy link
Member Author

vboykox commented Jun 22, 2020

Blocked until sonic-net/sonic-platform-common#96 is merged and sonic-buildimage updated correspondingly.

@lguohan
Copy link
Collaborator

lguohan commented Jul 2, 2020

sonic-net/sonic-platform-common#96 is merged. can you fix the lgtm alerts?

@vboykox
Copy link
Member Author

vboykox commented Jul 2, 2020

retest this please

@lgtm-com
Copy link

lgtm-com bot commented Jul 3, 2020

This pull request introduces 6 alerts when merging 01c471a0665fdaf6931ae14d09904418ac97110d into 4240c8c - view on LGTM.com

new alerts:

  • 4 for 'import *' may pollute namespace
  • 2 for Wrong number of arguments in a class instantiation

@vboykox
Copy link
Member Author

vboykox commented Jul 3, 2020 •
edited
Loading

  • 2 for Wrong number of arguments in a class instantiation

It looks like the check was done against corresponding constructors from other vendors' code, please ignore.

@vboykox vboykox force-pushed the bfn-paltform-api2.0-support branch from 63dcc60 to 45a37bc Compare July 6, 2020 10:20
@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2020

This pull request introduces 2 alerts when merging 45a37bc70d35b3b22574b68ced1be8aee6bd05de into 31baf38 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

@vboykox vboykox force-pushed the bfn-paltform-api2.0-support branch from d7b9db0 to 71adb99 Compare July 6, 2020 12:18
@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2020

This pull request introduces 3 alerts when merging 71adb99618623027e868d58dbf7e5996f75c03b2 into 31baf38 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation
  • 1 for Module imports itself

@vboykox vboykox force-pushed the bfn-paltform-api2.0-support branch from 71adb99 to e3c418b Compare July 6, 2020 12:33
@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2020

This pull request introduces 3 alerts when merging e3c418be621eff088805dbe05d9fc6c86f149abf into 31baf38 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation
  • 1 for Module imports itself

@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2020

This pull request introduces 3 alerts when merging d6722baabf9b2c7cf0f1b78322a567e4b4cd4ca6 into 31baf38 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation
  • 1 for Module imports itself

@vboykox vboykox force-pushed the bfn-paltform-api2.0-support branch from d6722ba to f8d65fb Compare July 6, 2020 14:11
@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2020

This pull request introduces 2 alerts when merging f8d65fbee6b15c401c6d3e7c6ee507eb6dfc5ad7 into 31baf38 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

@vboykox
Copy link
Member Author

vboykox commented Jul 10, 2020

@lguohan the PR has been updated

@vboykox vboykox force-pushed the bfn-paltform-api2.0-support branch 2 times, most recently from 7028f3b to 2caae5e Compare July 21, 2020 10:29
@lgtm-com
Copy link

lgtm-com bot commented Jul 21, 2020

This pull request introduces 2 alerts when merging 2caae5e8f516f07d915cbeb740840d033f2a6853 into 1870c18 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

@vboykox
Copy link
Member Author

vboykox commented Jul 21, 2020

@lguohan

@lguohan
Copy link
Collaborator

lguohan commented Jul 25, 2020

can you help to explain the alerts "2 for Wrong number of arguments in a class instantiation"?

@vboykox
Copy link
Member Author

vboykox commented Jul 26, 2020

  • 2 for Wrong number of arguments in a class instantiation

It looks like the check was done against corresponding constructors from other vendors' code, please ignore.

@lguohan False positive; the warning was also ignored for other verndors' PRs

@vboykox
Copy link
Member Author

vboykox commented Jul 28, 2020

@lguohan could you please merge

@akokhan
Copy link
Contributor

akokhan commented Aug 31, 2020

@lguohan, please approve and merge

bratashX and others added 2 commits September 7, 2020 00:36
@vboykox
Copy link
Member Author

vboykox commented Sep 7, 2020

@lguohan could you please give a reply?

@lgtm-com
Copy link

lgtm-com bot commented Sep 7, 2020

This pull request introduces 2 alerts when merging b87aac6 into 8d285b4 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

@vboykox
Copy link
Member Author

vboykox commented Sep 10, 2020

@lguohan please merge

@vboykox
Copy link
Member Author

vboykox commented Sep 22, 2020

@lguohan @jleveque

@jleveque
Copy link
Contributor

@vboykox; Please fix conflicts.

@lgtm-com
Copy link

lgtm-com bot commented Sep 23, 2020

This pull request introduces 2 alerts when merging 9d05a27 into 418e437 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

@vboykox
Copy link
Member Author

vboykox commented Sep 23, 2020

@jleveque done

@lgtm-com
Copy link

lgtm-com bot commented Sep 23, 2020

This pull request introduces 2 alerts when merging 502984b80ab3ad59c20a9d4294ba09ae1b7a1609 into 418e437 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

jleveque
jleveque suggested changes Sep 23, 2020
View reviewed changes
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.

There are still LGTM alerts. Please fix.

@vboykox
Copy link
Member Author

vboykox commented Sep 23, 2020

  • 2 for Wrong number of arguments in a class instantiation

It looks like the check was done against corresponding constructors from other vendors' code, please ignore.

@lguohan False positive; the warning was also ignored for other verndors' PRs

@jleveque

@vboykox
Copy link
Member Author

vboykox commented Sep 23, 2020

#4552 (comment)

@lguohan please checkout the comment from merged PR.

@jleveque

@jleveque
Copy link
Contributor

Thanks for pointing that out. Looks like the LGTM tool needs some imporvements :)

@lgtm-com
Copy link

lgtm-com bot commented Sep 24, 2020

This pull request introduces 2 alerts when merging 912676c into 623d5c0 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Oct 1, 2020

This pull request introduces 2 alerts when merging 2790dff into 1f0f751 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Oct 1, 2020

This pull request introduces 2 alerts when merging 8a2b534 into 1f0f751 - view on LGTM.com

new alerts:

  • 2 for Wrong number of arguments in a class instantiation

@vboykox vboykox requested a review from jleveque October 1, 2020 16:32
@jleveque
Copy link
Contributor

jleveque commented Oct 3, 2020

@vboykox: Thank you very much for helping us prepare for the transition to Python 3!

@jleveque jleveque merged commit dbea3bb into sonic-net:master Oct 3, 2020
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
@vboykox @santhosh-kt
BFN platform API 2.0 support (sonic-net#4766)
011facc 
Added barefoot platform api 2.0 support

Signed-off-by: Volodymyr Boyko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jleveque jleveque jleveque approved these changes

Assignees
No one assigned
Labels
Enhancement ➕ Platform: Barefoot
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vboykox @akokhan @lguohan @jleveque @bratashX