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

Implemented FarasaDepParser #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZOUHEIRBN
Copy link

Included dependency parsing as a new module, with binary JAR downloading function from release

@MagedSaeed
Copy link
Owner

MagedSaeed commented Aug 19, 2024

Salam @ZOUHEIRBN

Thanks for your nice contribution. I have some comments to discuss..

  • I see you are putting farasa binaries in your fork in GitHub releases. This is a license violation and we have an agreement from farasa team to only consider downloading the farasa_bin from the URL they are providing. I would really advice not to continue in this direction.

In the meanwhile, I thought [following this PR: https://github.com//pull/26] to give the ability to add a downloaded JAR for tasks that are not available in farasa_bin. So that the user can submit the request to get the JAR file from the farasa main site, get the JAR, then add its path to the task he is interested in and supported in farasapy, What do you think?

@ZOUHEIRBN
Copy link
Author

Hello sir @MagedSaeed,

Thank you for considering my contribution! I think adding the ability to add custom JAR modules as a feature would provide a lot more flexibility to the program, which is in my opinion a great attribute! Especially since including the binaries in the release would be a license violation.

@MagedSaeed
Copy link
Owner

@ZOUHEIRBN , can you take a look into this PR?

#27

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

Successfully merging this pull request may close these issues.

2 participants