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

discord-ptb: update to 0.0.22 #24117

Merged
merged 1 commit into from Sep 12, 2020
Merged

Conversation

MGlolenstine
Copy link
Contributor

No description provided.

srcpkgs/discord-ptb/template Outdated Show resolved Hide resolved
@fosslinux
Copy link
Contributor

Thanks for your PR!

https://travis-ci.org/github/void-linux/void-packages/jobs/715664522 as you can see here, there are some xlint erorrs. Would you mind fixing them?

@MGlolenstine
Copy link
Contributor Author

MGlolenstine commented Aug 7, 2020

On it!

EDIT: I've been trying to find a license Discord uses, but was unable to.
it's set as "proprietary" in the file, but no SPDX is set. Now I've found this https://discord.com/terms , but I'm unsure on how to add it to the file.
Here it states that it can be a Public Domain. Does that mean that I can just insert the URL from above and we're set?

@fosslinux
Copy link
Contributor

fosslinux commented Aug 7, 2020

The custom: speciefier indicates it is not SPDX. Use something along the lines of custom:proprietary.

The line wrap looks good, but maybe make it consistent. I.e., if you are going to split at (I think you did) 90 characters, split the next line at 90 characters too... keep all of the lines for that block the same length.

Eg:

	for item in DiscordPTB chrome_100_percent.pak chrome_200_percent.pak icudtl.dat libEGL.so \
	libGLESv2.so libffmpeg.so locales natives_blob.bin resources resources.pak snapshot_blob.bin swiftshader v8_context_snapshot.bin; do

to

	for item in DiscordPTB chrome_100_percent.pak chrome_200_percent.pak icudtl.dat libEGL.so \
	libGLESv2.so libffmpeg.so locales natives_blob.bin resources resources.pak snapshot_blob.bin \
        swiftshader v8_context_snapshot.bin; do

@ericonr
Copy link
Member

ericonr commented Aug 7, 2020

I believe "custom:Proprietary" is preferred. And line wraps around 80 columns, if possible.

@MGlolenstine
Copy link
Contributor Author

I think "custom:Proprietary" had some problems with xlint and was throwing errors... I'll have to check out that tomorrow (it's midnight here) and I'm in for 80 char line wrap.

@MGlolenstine
Copy link
Contributor Author

@ericonr This is the lint error I was talking about. I'm unsure what to put at the vlicense if anything.

@ericonr
Copy link
Member

ericonr commented Aug 8, 2020

You might need to find an EULA or License Agreement. The spotify template, for example, has to do a similar dance.

@MGlolenstine
Copy link
Contributor Author

You might need to find an EULA or License Agreement. The spotify template, for example, has to do a similar dance.

Would this do?
How should I format it in the file?

vlicense=https://discord.com/terms

@ericonr
Copy link
Member

ericonr commented Aug 8, 2020

I think that file would work, but you'd have to do something like https://github.com/void-linux/void-packages/blob/master/srcpkgs/spotify/template#L23 to pre-process it.

@MGlolenstine
Copy link
Contributor Author

I think that file would work, but you'd have to do something like https://github.com/void-linux/void-packages/blob/master/srcpkgs/spotify/template#L23 to pre-process it.

Oh wow, that looks complicated... It's pretty late here, so I'll take my time tomorrow and modify it...

@MGlolenstine
Copy link
Contributor Author

Would this be ready for merge, or is there something still missing?

@FirstZero
Copy link
Contributor

Ideally you might want to squash rebase your commits into one. Other than that, this seems ready for review.

@ericonr
Copy link
Member

ericonr commented Aug 14, 2020

You need to squash these commits before it can be merged.

@MGlolenstine
Copy link
Contributor Author

Ugh... I guess that squash didn't work like I wanted it to. 😅

@MGlolenstine
Copy link
Contributor Author

This seems better.

srcpkgs/discord-ptb/template Outdated Show resolved Hide resolved
srcpkgs/discord-ptb/template Outdated Show resolved Hide resolved
@chilledfrogs
Copy link
Contributor

BTW, just a little detail, but they moved to https://discord.com/ recently-ish @MGlolenstine, might wanna update that in homepage (for the moment the old URL is still usable though so not a huge deal)

@MGlolenstine
Copy link
Contributor Author

BTW, just a little detail, but they moved to https://discord.com/ recently-ish @MGlolenstine, might wanna update that in homepage (for the moment the old URL is still usable though so not a huge deal)

I've just updated the URL.

@MGlolenstine
Copy link
Contributor Author

I've updated the package to 0.0.22.
Should I close this PR and create a new one, as the version changed or would it be ok, if I just changed the commit message and PR's title?

@fosslinux
Copy link
Contributor

No, you should just change the commit message and the title.

@MGlolenstine
Copy link
Contributor Author

Should commit message contain every change or just the "discord-ptb: update to 0.0.22"?

@MGlolenstine MGlolenstine changed the title discord-ptb: update to 0.0.21 discord-ptb: update to 0.0.22 Sep 10, 2020
@fosslinux
Copy link
Contributor

How you have it now is correct.

@Chocimier
Copy link
Member

@MGlolenstine Do you run this updated 0.0.22 version? Is everything ok?

@MGlolenstine
Copy link
Contributor Author

@Chocimier Correct, I've updated it because I needed it for my laptop. I'm running it and it's working perfectly fine.

@Chocimier Chocimier merged commit ee4418c into void-linux:master Sep 12, 2020
@MGlolenstine MGlolenstine deleted the discord-ptb branch November 26, 2020 10:47
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants