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

Remove unused imports #306

Merged
merged 2 commits into from
Oct 30, 2024
Merged

Remove unused imports #306

merged 2 commits into from
Oct 30, 2024

Conversation

Timmmm
Copy link
Contributor

@Timmmm Timmmm commented Oct 27, 2024

Remove unused Python import statements.

c_utils.py Outdated Show resolved Hide resolved
go_utils.py Outdated Show resolved Hide resolved
@aswaterman
Copy link
Member

I don't have a strong opinion on the matter, so I'll let the two of you hash this out, and will follow whatever is ultimately decided.

Remove unused Python import statements.
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.63%. Comparing base (25c09e6) to head (938e6d5).
Report is 4 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (25c09e6) and HEAD (938e6d5). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (25c09e6) HEAD (938e6d5)
2 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #306       +/-   ##
===========================================
- Coverage   95.87%   42.63%   -53.24%     
===========================================
  Files          10       10               
  Lines         752      720       -32     
===========================================
- Hits          721      307      -414     
- Misses         31      413      +382     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Timmmm
Copy link
Contributor Author

Timmmm commented Oct 29, 2024

@IIITM-Jay do you have any outstanding objections to this? As I understand it your comment was related to the # from shared_utils import ... comment which this PR doesn't touch.

@Timmmm Timmmm mentioned this pull request Oct 29, 2024
@IIITM-Jay
Copy link
Member

IIITM-Jay commented Oct 30, 2024

@Timmmm No, I don't have any objections regarding the PR. Just a small thing, if possible, otherwise I could also take that up in a separate PR if you wish - could you please remove the wildcard imports and import only the necessary and sufficient methods of shared module, if it's not that much work from your end, otherwise I am ok to do that.

As for a single work we should not include two statements one with commented out and other as wildcard.

Let me know whatever suits you!

@IIITM-Jay
Copy link
Member

IIITM-Jay commented Oct 30, 2024

@Timmmm and @aswaterman I have removed the unwanted commented line form the files included in this PR, for importing methods from shared module from the top and kept Wildcard imports for now. Will raise a separate PR for importing all the required and necessary methods/ definition only from that module instead of wildcard imports.

@Timmmm
Copy link
Contributor Author

Timmmm commented Oct 30, 2024

Sounds good to me. I squashed your commits for a slightly neater history. Let's resolve the wildcard imports in another PR like you say. (I can do it if you like; VSCode makes it very easy.)

@aswaterman aswaterman merged commit f73c2a4 into riscv:master Oct 30, 2024
2 checks passed
@Timmmm Timmmm deleted the unused_imports branch October 31, 2024 11:41
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.

4 participants