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

refactor: Update types syntax in cve_bin_tool/*.py #2375

Closed
terriko opened this issue Nov 21, 2022 · 7 comments
Closed

refactor: Update types syntax in cve_bin_tool/*.py #2375

terriko opened this issue Nov 21, 2022 · 7 comments
Labels
good first issue Good for newcomers

Comments

@terriko
Copy link
Contributor

terriko commented Nov 21, 2022

We're in the middle of trying to improve the type hints we use in cve-bin-tool. We had been using an older syntax for backwards compatibility reasons, but since it turns out we can use modern syntax using from __future__ import annotations even on the oldest version of python we support (python 3.7) it makes more sense to modernize now.

The old types are usually handled using the typing library so you can find most of the old-style files using

grep -r "from typing import" .

What we need to do in each file is change that from typing import [...] line into from __future__ import annotations and then update the syntax accordingly in the rest of the file. The list of things on the import line should give you a hint for what you need to search and replace. For example, if your from typing line says from typing import Dict, List you'll need to replace every instance of Dict and List with newer types. In most cases, it's going to be the same thing but lowercase, but some of the changes require a bit more thought. Read the Python typing documentation if you're unsure what to do, or feel free to ask for help here or in your pull request.

You can and should run mypy on the file to check your types. pip install mypy if you don't already have it installed, and you may want to run pip install -r dev-requirements.txt and mypy --install-types to get all the type data we're currently using. Eventually we'll be adding mypy to our linter tests but we haven't enabled it yet, so you'll need to do this manually. Feel free to resolve any other mypy issues you find at the same time.

Here's a list of files that probably need upgraded types in this group:

cve_bin_tool/async_utils.py:from typing import Optional
cve_bin_tool/cli.py:from typing import Dict
cve_bin_tool/config.py:from typing import Any, Mapping
cve_bin_tool/csv2cve.py:from typing import List, Optional
cve_bin_tool/cve_scanner.py:from typing import DefaultDict, Dict, List, Tuple, Union
cve_bin_tool/cvedb.py:from typing import Any
cve_bin_tool/error_handler.py:from typing import Union
cve_bin_tool/format_checkers.py:from typing import List
cve_bin_tool/input_engine.py:from typing import Any, DefaultDict, Dict, Iterable, Set, Union
cve_bin_tool/merge.py:from typing import Dict, List
cve_bin_tool/nvd_api.py:from typing import Dict, List, Union
cve_bin_tool/package_list_parser.py:from typing import Any, Dict, List
cve_bin_tool/strings.py:from typing import ClassVar, List, Set
cve_bin_tool/util.py:from typing import DefaultDict, Iterator, List, NamedTuple, Pattern, Set, Union
cve_bin_tool/version_scanner.py:from typing import Iterator
cve_bin_tool/version_signature.py:from typing import Union

Like I did with hacktoberfest, I'm filing a few issues for different groups of files so multiple new contributors can work on this without interfering with each others work. If you want to "claim" the bug please leave a comment below saying that you'll work on it. You do not need to wait for us to assign the bug to you; if you're the first commenter just go ahead and start making a fix. I will try to give first commenters priority for merging unless they take more than a week to get a PR up after commenting. (If it takes more than a week either comment that you're still trying or we'll assume you have moved on and the bug will be open for someone else to try.) (Although I'm grouping the files, it's fine if you want to make a PR for each file separately, I'm just trying to reduce the amount of time I spend on cut and pasting the instructions.)

Helpful resources:

Tiny tips for new contributors:

  • cve-bin-tool uses https://www.conventionalcommits.org/ style for commit messages, and we have a test that checks the title of your PR. A good potential title for this one is in the title of this issue.
  • You can make an issue auto close by including a comment "fixes #ISSUENUMBER" in your PR comments where ISSUENUMBER is the actual number of the issue.
@terriko terriko added the good first issue Good for newcomers label Nov 21, 2022
@rudychung
Copy link
Contributor

Hi! I'd like to start working on this.

@Dev2907
Copy link

Dev2907 commented Feb 10, 2023

hello! i would like to contribute to this
however i am new to open source contributions
is this still open?

Dev2907 added a commit to Dev2907/cve-bin-tool that referenced this issue Feb 17, 2023
Refactor : Update types syntax in cve_bin_tool/*.py intel#2375
@varsha089
Copy link

Hello, @terriko I am a GSOC'23 aspirant and want to start contributing with a good first issue. I would like to contribute to this issue, please assign it to me.

@terriko
Copy link
Contributor Author

terriko commented Feb 21, 2023

@varsha089 we don't assign issues in this repo, just go ahead and run mypy on those files and see if any issues remain. If there are none left, please comment here so this issue can be closed.

Edit: actually it looks like there's another PR above that may fix all remaining issues. I'm running tests on it now, but I'd assume it will handle them.

@varsha089
Copy link

@terriko, sorry I was not active for the last 5 days because of my university exam, Now my examinations are over, and wanted to concentrate on the project understanding and making contributions.

Thanks for your reply,
as this issue is going to close, could you please assign me another issue to work on, that can help me to understand the project and its workflow?
Thank you.

@terriko
Copy link
Contributor Author

terriko commented Feb 28, 2023

@varsha089 I don't actually assign issues to people at all, but the contributors docs gives some suggestions on how to get started that might help you figure it out on your own https://github.com/intel/cve-bin-tool/blob/main/CONTRIBUTING.md#where-should-i-start (and note the section below the one I linked explaining about claiming issues and why I don't assign them to individuals.)

If there's nothing marked "good first issue" you'll also learn a lot from taking one of the other issues and giving it a shot. there's over a hundred to choose from! Good luck!

And I feel I should warn you: some GSoC projects, this one included, expect a lot of independent decision making. If you're in a stage in life where being told to look through a list of issues and just pick something is extremely daunting and you want more of a checklist of things to do assigned to you personally (and that's fine! Sometimes that's what you need!) you may want to look at other projects now and try to find one which has more mentors or clearer paths available to you. We are not going to be a good project for someone who needs that level of direction from mentors. No hard feelings if that turns out to be the case! You should absolutely shop around for a project that fits your style now while we're early in the process.

@terriko terriko closed this as completed in 0d3fbc0 Mar 1, 2023
@varsha089
Copy link

@terriko I am extremely sorry for my immature behavior toward the project I appreciate your encouragement to take on a challenging issue, even if it's not marked as a "good first issue." I understand that independent decision-making is an important aspect of this project, and I am prepared to take on that responsibility.
Thank you also for your honesty and transparency about the level of mentorship and direction available for this project.
I appreciate the opportunity to explore this project and learn more about it. Thank you again for your time and guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants