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

Contributors needed? #93

Open
ArjanKw opened this issue Aug 14, 2024 · 7 comments
Open

Contributors needed? #93

ArjanKw opened this issue Aug 14, 2024 · 7 comments

Comments

@ArjanKw
Copy link

ArjanKw commented Aug 14, 2024

At the moment there are 4 helpful open PRs (#54, #55, #90 and #92) from people who would like to advance this project, adding .NET 8 support, unit tests, wkhtmltoimage support, new properties, documentation and configuration. They are dating back to 2020. Do you need help in maintaining this project? I'm using this project, so I would be happy to contribute.

@webgio
Copy link
Owner

webgio commented Aug 14, 2024 via email

@ArjanKw
Copy link
Author

ArjanKw commented Aug 14, 2024

Thank you for your reaction @webgio and take your time! I could help by keeping the project up-to-date with new .NET versions, adding more documentation and responding to issues/PRs if able. If you like to have the final say in changes I could let you do the merges.

The #92 PR contains the changes proposed in the other PRs as well, as I needed both .NET 8 and image support, while refactoring the code a bit to prevent code duplication. I added the Zoom property as well because there was a conflict in that PR. The PR also solves #88. Issue #17 and #85 can also be closed after the merge because we allow people to disable setting the base URL. So we could close 4 PRs and 3 issues by reviewing and merging 1 PR. In the future I need to make the PRs smaller.

@ArjanKw
Copy link
Author

ArjanKw commented Oct 19, 2024

Hi @webgio, how would you like to go forward? Will you have time for some necessary maintenance in the near future so this project could stay relevant? If yes, that would be perfect, if no, that is understandable. If you don't have the time anymore, could you make a decision on how this project could stay up-to-date without your input? It could also be a temporary solution up until when you have time again.

I don't want to fork this project, forcing people to switch packages and ending up with two development paths if you continue later. I'm happy with any direction you choose that would keep the project going.

In the future I would like to add additional drivers to this project, as wkhtmltopdf is archived and no longer maintained. Then this project will make it easy to add PDF capabilities to any .NET project, with a PDF generator of your choice.

@webgio
Copy link
Owner

webgio commented Oct 19, 2024

Hi @ArjanKw, I had issues reviewing your PR, could you check my comments? I don't have much time but I think we can merge the PR pretty quickly if I'm able to run it locally.

@ArjanKw
Copy link
Author

ArjanKw commented Oct 19, 2024

Thanks @webgio. I don't see any comments on the #92 PR, or a review. Where could I find the comments? I would be more than happy to address the issues to get it running locally on your PC.

@webgio
Copy link
Owner

webgio commented Oct 19, 2024

@ArjanKw I thought the comments were visible, sorry. I started the review in August... so I'm trying to get back to it and remember what was the point of the issue. I was trying to run tests but I couldn't understand how you run both .net 6 and .net 8 versions on different ports. I think everything else looks good.

@ArjanKw
Copy link
Author

ArjanKw commented Oct 20, 2024

No problem. In line with the rest of the code I commented out the Asp.net 6 variant and added the Asp.net 8 variant. The Github review makes it seem in the review page as if both Asp.net 6 and Asp.net 8 are not commented, but this is the end result:

//[InlineData("http://localhost:64310", "Asp.net core 2.0")]
//[InlineData("https://localhost:44375", "Asp.net core 3.1")]
//[InlineData("https://localhost:7059", "Asp.net 6")]
[InlineData("https://localhost:56246", "Asp.net 8")]

56246 is the port where the DemoApp is running on locally on my machine. That is different for everyone because the launchSettings.json is an ignored file in Git. If you want the port to be the same for everyone, we could unignore the launchSettings.json file.

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

No branches or pull requests

2 participants