Skip to content
This repository has been archived by the owner on Jun 16, 2022. It is now read-only.

Bugfix: Solana linting #4902

Merged
merged 8 commits into from
Apr 11, 2022
Merged

Bugfix: Solana linting #4902

merged 8 commits into from
Apr 11, 2022

Conversation

lambertkevin
Copy link
Contributor

🦒 Context (issues, jira)

The integration of Solana has some linting issues. This PR should fix the errors.

🖤 Expectations to reach

PR must pass CI, rebase develop if conflicts. Thanks!

  • on QA: at least one of these two checkboxes must be checked:
    • a specific test planned is defined on Jira
    • this PR is covered by automatic UI test
  • on delivery: at least one of these two checkboxes must be checked:
    • Option 1: no impact: The changes of this PR have ZERO impact on the userland (invisible for users)
    • Option 2: atomic delivery: the changes is atomic and complete (no partial delivery)

@lambertkevin lambertkevin requested review from a team as code owners April 7, 2022 20:44
@lambertkevin lambertkevin changed the base branch from develop to release/2.41.x April 7, 2022 20:45
@github-actions
Copy link

github-actions bot commented Apr 7, 2022

Thanks for your contribution.
To be able to merge in develop branch, you need to:

  • pass the CI
  • if needed, run /generate-screenshots
  • have a dev review
  • have a QA review
  • if needed, /upgrade-llc

Why /generate-screenshots ?

If your PR contains UI related changes,
it might be necessary to regenerate screenshots.

Why /upgrade-llc ?

If your PR requires an update to the ledger-live-common library,
once the PR is merged on develop on ledger-live-common side,
you need to run /upgrade-llc to switch back to ledger-live-common@develop here before merging.

lambertkevin
lambertkevin commented Apr 7, 2022
View reviewed changes
Copy link
Contributor Author

@lambertkevin lambertkevin left a comment

Choose a reason for hiding this comment

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

Fixed the linting + an optional component.

Still needs to be tested cause I removed a lot of "unused" code and I don't know if it's normal or not.

src/renderer/families/solana/shared/fields/ValidatorsField.js Show resolved Hide resolved
src/renderer/families/solana/shared/fields/ValidatorsField.js Outdated Show resolved Hide resolved
@lambertkevin lambertkevin added the important label used to flag a PR as important to be merged in priority (sometimes because dependencies) label Apr 7, 2022
@github-actions
Copy link

github-actions bot commented Apr 7, 2022 •
edited
Loading

Coverage report

St.❔
 Category Percentage Covered / Total
🔴 Statements 8.74% 9/103
🔴 Branches 0% 0/19
🔴 Functions 2.94% 1/34
🔴 Lines 8.33% 8/96

Test suite run success

1 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from 9ea0dd9

ghost
ghost approved these changes Apr 8, 2022
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Code LGTM. Thanks a lot!
Why no CI though

src/renderer/families/solana/shared/fields/ValidatorsField.js Show resolved Hide resolved
@lambertkevin lambertkevin added the HODL for PRs: this is blocked, we can't merge yet label Apr 8, 2022
@lambertkevin lambertkevin changed the base branch from release/2.41.x to develop April 8, 2022 07:33
ghost
ghost approved these changes Apr 8, 2022
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

All good at last, just need to target release/2.41.x instead of develop.

@ghost
Copy link

ghost commented Apr 8, 2022

/generate-screenshots

@github-actions
Copy link

github-actions bot commented Apr 8, 2022

Generating new screenshots

@lambertkevin lambertkevin changed the base branch from develop to release/2.41.x April 11, 2022 11:16
@lambertkevin lambertkevin merged commit dd6b8d5 into release/2.41.x Apr 11, 2022
@lambertkevin lambertkevin deleted the bugfix/solana-linting branch April 11, 2022 11:17
lambertkevin added a commit that referenced this pull request Apr 11, 2022
@lambertkevin @hakim-adamik
Bugfix: Solana linting (#4902)
8e47227 
* Fix Solana linting

* Add back null condition to SolanaValidatorField while respecting React "no conditional hook" rule

* Fix more lint

* Silent flow error

* update LLC dependency

Co-authored-by: haammar-ledger <[email protected]>
lambertkevin added a commit that referenced this pull request Apr 11, 2022
@lambertkevin @hakim-adamik
Bugfix: Solana linting (#4902)
d4957e3 
* Fix Solana linting

* Add back null condition to SolanaValidatorField while respecting React "no conditional hook" rule

* Fix more lint

* Silent flow error

* update LLC dependency

Co-authored-by: haammar-ledger <[email protected]>
lambertkevin added a commit that referenced this pull request Apr 12, 2022
@lambertkevin @hakim-adamik
Bugfix: Solana linting (#4902)
a4adeca 
* Fix Solana linting

* Add back null condition to SolanaValidatorField while respecting React "no conditional hook" rule

* Fix more lint

* Silent flow error

* update LLC dependency

Co-authored-by: haammar-ledger <[email protected]>
lambertkevin added a commit that referenced this pull request Apr 20, 2022
@lambertkevin @hakim-adamik
Bugfix: Solana linting (#4902)
dff0e73 
* Fix Solana linting

* Add back null condition to SolanaValidatorField while respecting React "no conditional hook" rule

* Fix more lint

* Silent flow error

* update LLC dependency

Co-authored-by: haammar-ledger <[email protected]>
valpinkman added a commit that referenced this pull request May 3, 2022
@valpinkman
Merge branch 'release/2.41.x'
c2ce96d 
* release/2.41.x: (56 commits)
  v2.41.3
  Fix NFT send making the app crash in prod (again) (#4958)
  v2.41.2
  Update to LLC 22.0.3
  FIX LIVE-2186 - Fix NFT crashing the app on send (#4953)
  Remove @ledgerhq/ledger-core again after bad merge (#4951)
  Update to temp LLC dependency + @LedgerHQ libs
  FIX LIVE-2176 - Fix NFT Crumb (#4950)
  v2.41.1
  Update LLC dependency
  LIVE-2080 Fixing Flowtype issues being wrongly ignored (#4938)
  v2.41.0
  update screenshots (windows)
  update screenshots (linux)
  New Crowdin updates (#4913)
  Add LLC#22.0.1 dependency
  Update links to use the new explorer from metadata (#4912)
  Bugfix: Solana linting (#4902)
  [LIVE-1911] Feature - Add collection name resolver to eth familly (#4899)
  LIVE-1751 Solana staking (#4884)
  ...
valpinkman added a commit that referenced this pull request May 3, 2022
@valpinkman
Merge branch 'release/2.41.x' into develop
1d0dfc3 
* release/2.41.x: (31 commits)
  v2.41.3
  Fix NFT send making the app crash in prod (again) (#4958)
  v2.41.2
  Update to LLC 22.0.3
  FIX LIVE-2186 - Fix NFT crashing the app on send (#4953)
  Remove @ledgerhq/ledger-core again after bad merge (#4951)
  Update to temp LLC dependency + @LedgerHQ libs
  FIX LIVE-2176 - Fix NFT Crumb (#4950)
  v2.41.1
  Update LLC dependency
  LIVE-2080 Fixing Flowtype issues being wrongly ignored (#4938)
  v2.41.0
  update screenshots (windows)
  update screenshots (linux)
  New Crowdin updates (#4913)
  Add LLC#22.0.1 dependency
  Update links to use the new explorer from metadata (#4912)
  Bugfix: Solana linting (#4902)
  [LIVE-1911] Feature - Add collection name resolver to eth familly (#4899)
  LIVE-1751 Solana staking (#4884)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
Assignees
No one assigned
Labels
HODL for PRs: this is blocked, we can't merge yet important label used to flag a PR as important to be merged in priority (sometimes because dependencies)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lambertkevin @hakim-adamik