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

SolidityScan into solidity compiler #4908

Merged
merged 18 commits into from
Jun 24, 2024
Merged

SolidityScan into solidity compiler #4908

merged 18 commits into from
Jun 24, 2024

Conversation

Aniket-Engg
Copy link
Collaborator

@Aniket-Engg Aniket-Engg commented Jun 20, 2024

  • Option to run classic static analysis and solidity scan now added in Solidity compiler plugin
  • Result of Solidity scan appears in Remix terminal

Copy link

netlify bot commented Jun 20, 2024

Deploy Preview for remixproject ready!

Name Link
🔨 Latest commit 5f8fffb
🔍 Latest deploy log https://app.netlify.com/sites/remixproject/deploys/6679779f707cdc00088b8a19
😎 Deploy Preview https://deploy-preview-4908--remixproject.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Aniket-Engg Aniket-Engg changed the title show solscan result details in terminal SolidityScan into solidity compiler Jun 20, 2024
@Aniket-Engg Aniket-Engg added the ready-to-review PR ready to review label Jun 20, 2024
@LianaHus
Copy link
Collaborator

LianaHus commented Jun 20, 2024

  • The titles you use for Btns sound more like a tooltip to me. can we get rid of "run"?
  • we need icons for newly added bins. SA has one, for Sol scan, you can ask them maybe
  • The first btns promises to run analytics, but it is enabling the plugin only. It should enable with await and run on the current compiled contract

@Aniket-Engg
Copy link
Collaborator Author

The titles you use for Btns sound more like a tooltip to me. can we get rid of "run"?

I disagree with this. That will make it less user friendly. Also other buttons are having Publish

@Aniket-Engg
Copy link
Collaborator Author

The first btns promises to run analytics, but it is enabling the plugin only. It should enable with await and run on the current compiled contract

No APIs are exposed in SSA plugin to run analysis like that. For now, I move users to SSA plugin to run analysis there

@Aniket-Engg
Copy link
Collaborator Author

Aniket-Engg commented Jun 20, 2024

we need icons for newly added bins. SA has one, for Sol scan, you can ask them maybe

Added icon for SSA, asked for solscan one

@Aniket-Engg Aniket-Engg requested a review from yann300 June 21, 2024 06:07
@yann300
Copy link
Contributor

yann300 commented Jun 21, 2024

LGTM!

@LianaHus
Copy link
Collaborator

  • you have issues with dark icons in dark theme
  • we may consider moving icons to the left as it is everywhere else in the ide
  • only the last one has no icon. we could add
  1. <i class="fa-regular fa-memo-circle-info"></i>
    or
  2. <i class="fa-regular fa-memo-pad"></i>

@Aniket-Engg Aniket-Engg added the solidity compiler plugin Related to Solidity Compiler Plugin label Jun 24, 2024
@LianaHus
Copy link
Collaborator

LGTM

@Aniket-Engg Aniket-Engg enabled auto-merge (rebase) June 24, 2024 13:37
@Aniket-Engg Aniket-Engg merged commit ad904db into master Jun 24, 2024
32 checks passed
@Aniket-Engg Aniket-Engg deleted the solsTerm branch June 24, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-review PR ready to review solidity compiler plugin Related to Solidity Compiler Plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants