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

Updated doc strings to version_scanner.py #3417

Closed
wants to merge 2 commits into from

Conversation

rexdivakar
Copy link

@rexdivakar rexdivakar commented Oct 14, 2023

Kindly review the PR and approve it, Issue #3393

@rexdivakar rexdivakar changed the title Updated doc strings to version_scanner.py #3378 Updated doc strings to version_scanner.py Oct 14, 2023
@rexdivakar
Copy link
Author

Hi @terriko Kindly validate and approve the PR

@terriko
Copy link
Contributor

terriko commented Oct 16, 2023

I've approved the tests to run, but it looks like something went wrong with the indentation. It's unlikely we'll be able to merge this until that's fixed. Maybe a bad setting in your development tools?

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Marking as needing changes due to indentation problems.

@codecov-commenter
Copy link

Codecov Report

Merging #3417 (4827f36) into main (56fc482) will decrease coverage by 0.29%.
Report is 2 commits behind head on main.
The diff coverage is 84.07%.

@@            Coverage Diff             @@
##             main    #3417      +/-   ##
==========================================
- Coverage   80.21%   79.93%   -0.29%     
==========================================
  Files         758      758              
  Lines       11596    11596              
  Branches     1568     1568              
==========================================
- Hits         9302     9269      -33     
- Misses       1862     1907      +45     
+ Partials      432      420      -12     
Flag Coverage Δ
longtests 74.81% <82.16%> (-4.58%) ⬇️
win-longtests 77.92% <79.61%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
cve_bin_tool/version_scanner.py 85.31% <84.07%> (ø)

... and 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@terriko
Copy link
Contributor

terriko commented Oct 16, 2023

I'm going to mark this as blocked because someone else had already claimed #3378 and they've got a PR in flight already. If for some reason that one can't be merged I'll consider this one, but more likely this issue will be closed as a duplicate after that one gets merged.

Feel free to work on it for practice if that's helpful anyhow, but if you're hoping to get a merged PR for hacktoberfest you should probably work on an issue not claimed by someone else. We've got lots of issues and they don't need the hacktoberfest label to count! But I did put a hacktoberfest label on this unclaimed one if you're looking for somethign to do: #3255

@rexdivakar
Copy link
Author

I've approved the tests to run, but it looks like something went wrong with the indentation. It's unlikely we'll be able to merge this until that's fixed. Maybe a bad setting in your development tools?

I have been using pycharm for my local development, let me retry to re-indent it.

@terriko
Copy link
Contributor

terriko commented Oct 19, 2023

I've merged #3429 so this PR is no longer needed and I'm going to close it. But if you take a look at the strings in #3429 and think you'd like to add some information there that you learned while doing your own evaluation, feel free to open a new PR to tweak them.

@terriko terriko closed this Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants