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.

Sign up for GitHub

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

Jump to bottom

addpkg(main/languagetool-libs),addpkg(x11/languagetool-gui): languagetool 6.5 #22244

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

addpkg(main/languagetool-libs),addpkg(x11/languagetool-gui): languagetool 6.5 #22244

wants to merge 5 commits into from
+69 −0

Conversation

mbekkomo
Copy link
Contributor

Closes #21086

@mbekkomo mbekkomo force-pushed the newpkg-languagetool branch 3 times, most recently from 3cc6ebf to 0587b76 Compare November 13, 2024 08:55
@mbekkomo mbekkomo changed the title addpkg(main/languagetool),addpkg(x11/languagetool-gui): languagetool 6.5 addpkg(main/languagetool-libs),addpkg(x11/languagetool-gui): languagetool 6.5 Nov 13, 2024
@mbekkomo mbekkomo force-pushed the newpkg-languagetool branch 3 times, most recently from 61b7f13 to 921a0d3 Compare November 13, 2024 10:21
@mbekkomo mbekkomo marked this pull request as ready for review November 13, 2024 10:27
TomJo2000
TomJo2000 requested changes Nov 13, 2024
View reviewed changes
Copy link
Member

@TomJo2000 TomJo2000 left a comment

Choose a reason for hiding this comment

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

That's gonna have to be a NACK from me in the current state.

  • We're not building anything here, so the user may as well download it themselves.
  • The package(s) are gonna take up ~600MB of mirror space across all 4 architectures

packages/languagetool-libs/build.sh Outdated Show resolved Hide resolved
@TomJo2000
Copy link
Member

By the way does this not work with openjdk-21 or why isn't that an option?

@mbekkomo
Copy link
Contributor Author

@TomJo2000 Haven't tried it with openjdk-21, but I'll try it later.

@TomJo2000
Copy link
Member

TomJo2000 commented Nov 13, 2024 •
edited
Loading

You can use a pipe | character as a logical OR for dependencies.
See:

termux-packages/packages/yarn/build.sh

Lines 8 to 9 in 9558b2a

TERMUX_PKG_DEPENDS="nodejs | nodejs-lts"
TERMUX_PKG_ANTI_BUILD_DEPENDS="nodejs, nodejs-lts"

@mbekkomo mbekkomo force-pushed the newpkg-languagetool branch 3 times, most recently from 382472d to c53063d Compare November 13, 2024 12:24
truboxl
truboxl requested changes Nov 26, 2024
View reviewed changes
packages/languagetool-libs/languagetool.sh Outdated Show resolved Hide resolved
packages/languagetool-libs/languagetool.sh Outdated Show resolved Hide resolved
packages/languagetool-libs/languagetool.sh Outdated Show resolved Hide resolved
x11-packages/languagetool-gui/build.sh
TERMUX_PKG_SHA256="$(
source "${TERMUX_SCRIPTDIR}/packages/languagetool-libs/build.sh"
echo "${TERMUX_PKG_SHA256}"
)"
Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly disagree with this kind of sourcing version, URL, checksum. It breaks ./scripts/bin/revbump and maybe more scripts.

packages/languagetool-libs/build.sh Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomJo2000 TomJo2000 TomJo2000 requested changes

@truboxl truboxl truboxl requested changes

@Grimler91 Grimler91 Awaiting requested review from Grimler91 Grimler91 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Package]: languagetool
3 participants
@mbekkomo @TomJo2000 @truboxl