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

added html args #52

Closed
wants to merge 2 commits into from
Closed

added html args #52

wants to merge 2 commits into from

Conversation

FireNdIce3
Copy link

@FireNdIce3 FireNdIce3 commented Oct 2, 2022

I saw the code and did what I felt was right. If I am wrong somewhere, please guide me so I can solve this issue.
Thanks,

Fix #51


if ARGS.html is not None:
with open("report.html", "w") as e:
for lines in report_file.readlines():
Copy link
Contributor

Choose a reason for hiding this comment

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

Not working since your reading a file that is not closed yet (tested on python 3.10, may work elsewhere)
You'll have to close the report_file before, then reopen it in read mode and use report_file.read() simply

@Dynnammo
Copy link
Contributor

Dynnammo commented Oct 2, 2022

@FireNdIce3 first warm thanks for your contribution ! I made a quick review, you're not far from reaching a solution. message me if you need help.

@Mte90
Copy link
Owner

Mte90 commented Oct 3, 2022

I think also that there should be the repo anchor link in the HTML report not just a pre list of the various repositories.

@Mte90
Copy link
Owner

Mte90 commented Oct 10, 2022

Fixed the conflicts but @FireNdIce3 are you still interested on finish the PR?

@FireNdIce3
Copy link
Author

Yes, I am.. can you please give me a few days to fix the issue?
Thanks

@Mte90
Copy link
Owner

Mte90 commented Oct 11, 2022

yes no problem, I saw that you disappeared so I wasn't sure :-)

@Mte90
Copy link
Owner

Mte90 commented Oct 26, 2022

Ping as the hacktoberfest is finishing and if we need to tests to approve it we need that the PR is completed.

@Mte90
Copy link
Owner

Mte90 commented Nov 8, 2022

any updates?

@Dynnammo Dynnammo changed the title added html arg solves #51 added html args Nov 9, 2022
@Dynnammo
Copy link
Contributor

Dynnammo commented Nov 9, 2022

I'm trying to get some time on it this Friday, since it doesn't seem to require much time. Sorry for the delay, work sucked all my time since mid-october.

@FireNdIce3 FireNdIce3 closed this by deleting the head repository Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revamp the report file
3 participants