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

New package: sublime-text4-4107 #31036

Merged
merged 1 commit into from Sep 7, 2021
Merged

New package: sublime-text4-4107 #31036

merged 1 commit into from Sep 7, 2021

Conversation

DirectorX
Copy link
Contributor

@DirectorX DirectorX commented May 21, 2021 •
edited

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
  • I generally don't use the affected packages but briefly tested this PR

DirectorX
DirectorX commented May 21, 2021
View reviewed changes
srcpkgs/sublime-text4/template Outdated
maintainer="DirectorX <void.directorx@protonmail.com>"
license="custom:Proprietary"
homepage="https://www.sublimetext.com/"
replaces="sublime-text3>=0"
Copy link
Contributor Author

@DirectorX DirectorX May 21, 2021 •
edited

Choose a reason for hiding this comment

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

I didn't thought about how to deal with subl3 yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can just mark it as:

conflicts="sublime-text3"

@marmeladema marmeladema mentioned this pull request May 21, 2021
Closed
3 tasks
@DirectorX
Copy link
Contributor Author

aarch64 not tested

@ericonr ericonr added the new-package This PR adds a new package label May 21, 2021
@ericonr
Copy link
Member

ericonr commented May 22, 2021

Ideally should fix lint.

@jchook
Copy link
Contributor

jchook commented Jul 13, 2021

Would love to see this merged. From what I can see, the source tarball doesn't contain a license file, so I'm not sure how to resolve the lint error.

srcpkgs/sublime-text4/template:8: license 'custom', but no use of vlicense

@Nairou
Copy link
Contributor

Nairou commented Jul 13, 2021

Would it be sufficient to remove the license line in the template?

@jchook
Copy link
Contributor

jchook commented Jul 13, 2021

Apparently not.

srcpkgs/sublime-text4/template: 'license' missing!

paper42
paper42 reviewed Jul 13, 2021
View reviewed changes
srcpkgs/sublime-text4/template Outdated
pkgname=sublime-text4
version=4107
revision=1
depends="libpng gtk+ hicolor-icon-theme desktop-file-utils"
Copy link
Member

Choose a reason for hiding this comment

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

sublime-text should depend on GTK 3

paper42
paper42 reviewed Jul 13, 2021
View reviewed changes
srcpkgs/sublime-text4/template Outdated
Comment on lines 24 to 27
repository="nonfree"
archs="x86_64 aarch64"
wrksrc="sublime_text"
nopie=yes
Copy link
Member

Choose a reason for hiding this comment

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

why are these lines separate from the first block?

paper42
paper42 reviewed Jul 13, 2021
View reviewed changes
srcpkgs/sublime-text4/template Outdated
done
ln -s /usr/lib/sublime_text/sublime_text ${DESTDIR}/usr/bin/subl4
vsed -e 's:Exec=/opt/sublime_text/:Exec=/usr/lib/sublime_text/:' \
-i ${DESTDIR}/usr/share/applications/sublime_text.desktop
Copy link
Member

Choose a reason for hiding this comment

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

it would be nice to indent this line when it's a continuation of the previous one

@lopqto
Copy link

lopqto commented Jul 31, 2021

Hey folks,
any updates on this one?

@Chocimier
Copy link
Member

For vlicense, look at sublime-merge-bin.
Please sort lines as xlint says: https://github.com/leahneukirchen/xtools/blob/e778aefbcfd0e059f84540dfb90cdf64b4b1316b/xlint#L43

@Chocimier Chocimier mentioned this pull request Jul 31, 2021
Closed
3 tasks
@paper42 paper42 mentioned this pull request Aug 21, 2021
Closed
7 tasks
@Chocimier Chocimier mentioned this pull request Sep 1, 2021
Closed
3 tasks
@DirectorX
Copy link
Contributor Author

For vlicense, look at sublime-merge-bin.
Please sort lines as xlint says: https://github.com/leahneukirchen/xtools/blob/e778aefbcfd0e059f84540dfb90cdf64b4b1316b/xlint#L43

xlint says its A-OK

@Chocimier
Copy link
Member

Pretty good. Needs to be restricted however.
How is it working when installed?

@ashpool37
Copy link
Contributor

I've been using the package from this PR for a few weeks now on x86_64. Works for me.

@DirectorX
Copy link
Contributor Author

@Chocimier @ericonr I think I've done it

@Chocimier
Copy link
Member

Thanks!

@Chocimier Chocimier merged commit d22c148 into void-linux:master Sep 7, 2021
@DirectorX DirectorX deleted the sublime-text4 branch September 7, 2021 18:49
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marmeladema marmeladema marmeladema left review comments

@paper42 paper42 paper42 left review comments

Assignees
No one assigned
Labels
new-package This PR adds a new package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@DirectorX @ericonr @jchook @Nairou @lopqto @Chocimier @ashpool37 @marmeladema @paper42