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

feat: compiler settings #979

Merged
merged 11 commits into from
Aug 17, 2022
Merged

feat: compiler settings #979

merged 11 commits into from
Aug 17, 2022

Conversation

sabotagebeats
Copy link
Contributor

What I did

added get_compiler_settings

fixes: #

How I did it

How to verify it

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

@sabotagebeats sabotagebeats mentioned this pull request Aug 15, 2022
Closed
1 task
antazoey
antazoey reviewed Aug 15, 2022
View reviewed changes
Copy link
Member

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

I added some comments I hope are helpful!

src/ape/api/compiler.py Outdated Show resolved Hide resolved
src/ape/api/compiler.py Outdated Show resolved Hide resolved
src/ape/api/projects.py Outdated Show resolved Hide resolved
src/ape/api/projects.py Outdated Show resolved Hide resolved
@sabotagebeats
Copy link
Contributor Author

failing type check:

Run mypy .
src/ape/api/projects.py:153: error: Value of type String? is not indexable
src/ape/api/compiler.py:41: error: Variable "tokenize.String" is not valid as a type
src/ape/api/compiler.py:41: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
Found 2 errors in 2 files (checked 160 source files)
Error: Process completed with exit code 1.

sabotagebeats and others added 2 commits August 16, 2022 13:26
@sabotagebeats @fubuloubu
refactor: more performant return statement
41a30bb 
Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>
@sabotagebeats
chore: Merge branch 'main' of https://github.com/apeworx/ape into fea…
ceb9c10 
…t/compilersettings
@sabotagebeats
Copy link
Contributor Author

associated PR: ApeWorX/ape-solidity#64

@sabotagebeats
Copy link
Contributor Author

mypy fails if traitlets version is too small, also is being used and wasn't listed in setup.py

@sabotagebeats sabotagebeats marked this pull request as ready for review August 16, 2022 23:16
@sabotagebeats
Copy link
Contributor Author

adding tests in solidity plugin because that's where data will be found.

fubuloubu
fubuloubu reviewed Aug 17, 2022
View reviewed changes
setup.py
@@ -111,6 +111,7 @@
"singledispatchmethod ; python_version<'3.8'",
"tqdm>=4.62.3,<5.0",
"typing-extensions ; python_version<'3.8'",
"traitlets>=5.3.0",
Copy link
Member

Choose a reason for hiding this comment

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

Why was this added to dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

This is already being used in the ape-console plugin. However, we seem to have mypy issues when the version is 5.1.1 instead of 5.3

Copy link
Member

Choose a reason for hiding this comment

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

This is what Sab meant by this comment: #979 (comment)

antazoey
antazoey previously approved these changes Aug 17, 2022
View reviewed changes
Copy link
Member

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

This PR actually fixes an intermittent bug that appears across all plugins

src/ape/api/compiler.py Outdated Show resolved Hide resolved
src/ape/api/compiler.py Outdated Show resolved Hide resolved
antazoey
antazoey previously approved these changes Aug 17, 2022
View reviewed changes
@antazoey antazoey mentioned this pull request Aug 17, 2022
Merged
4 tasks
@sabotagebeats sabotagebeats merged commit 4bf1647 into ApeWorX:main Aug 17, 2022
@sabotagebeats sabotagebeats deleted the feat/compilersettings branch August 17, 2022 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fubuloubu fubuloubu fubuloubu left review comments

@antazoey antazoey antazoey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sabotagebeats @fubuloubu @antazoey