Closed Osiris-Team closed 3 months ago
Firstly, we'd like to thank you for taking the time and effort to contribute to our PDF Viewer component. We value and encourage community involvement in our projects, and your willingness to enhance our product is commendable.
After careful review of your Pull Request, however, we regret to inform you that we won't be able to accept it in its current state. We've identified several issues:
The Pull Request lacks clarity regarding the specific issue or feature it addresses. Providing clear context helps us understand the purpose and impact of the proposed changes better.
Documentation is missing. Proper documentation is crucial for maintaining and understanding the functionality of the component. The feature introduced in your PR requires more comprehensive documentation to help users in utilizing it effectively and to help maintenance tasks as well.
Commented code is present in the PR, which could add confusion to the codebase. It's best to remove such code entirely or provide explanations in comments if it's necessary for historical reasons.
The inclusion of unnecessary Jitpack references in the pom and readme files could lead to unnecessary complexity and potential conflicts. We aim to keep dependencies minimal and relevant to maintain a streamlined project structure.
Public variables should be avoided where possible to maintain encapsulation and adhere to best practices in object-oriented design.
It's unclear whether you considered the possibility of integrating this feature into the web component part of the PDF Viewer. Exploring this avenue could potentially reduce the need for additional HTML, JS files, and methods, enhancing the component's efficiency and maintainability.
We will proceed to close this PR and we encourage you to create the necessary github issue/s explaining the feature request and the proposed solution/s so it can be discussed and addressed accordingly.
Ah yeah, no worries, expected that, just wanted to try out anyways and see if you guys need these changes since this was a project made for a client of mine.
Integrating it into the web component part kind of really makes more sense.