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

New package: tspreed-2.0.1 #27113

Closed
wants to merge 1 commit into from
Closed

Conversation

astralchan
Copy link
Contributor

Additions

Adds the shell script "tspreed." I just read the manual that seemed a little over me, I am probably doing this wrong. This is my first time ever trying contributing to VOID.

@astralchan
Copy link
Contributor Author

I'm not sure why the Lint templates check is failing. It is released under GPL license as seen here, and "GPL-3.0-or-later" should be the right license as seen here.

@ericonr
Copy link
Member

ericonr commented Dec 12, 2020

It is complaining that the license is being installed with vlicense, not about the value.

Also, please squash your commits and use the appropriate commit message New package: tspreed-${version}

@astralchan
Copy link
Contributor Author

Umm, I tried squashing the commits and now there's more files changed for some reason. I just want to change the tspreed file (I'm not a master of git...)

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

Umm, I tried squashing the commits and now there's more files changed for some reason. I just want to change the tspreed file (I'm not a master of git...)

Switch to master and do git pull upstream(assuming you have https://github.com/void-linux/void-packages as upstream) master --rebase ; git push origin master

In this branch git pull upstream master --rebase ; though I don't know if it will fix if you squashed any others that doesn't belong with this commit.

@astralchan
Copy link
Contributor Author

Okay, looks like its back to just adding the tspreed template. I still am not sure how to fix Lint check failure.

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

Remove vlicense line. Common licenses don't need it, licenses such as MIT, BSD* for example does need it

@astralchan
Copy link
Contributor Author

Removed vlicense line

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

To squash your commits, do: git rebase -i HEAD~5 and the commits related to this package, first commit change p to r (reword), other 3 change p to f (fixup). When editor comes up change commit message to: New package: tspreed-1.2.1 , save and then git rebase --continue

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

Error is that you need to change the very first line to: # Template file for 'tspreed' instead of # Template file for 'teespreed'

@astralchan
Copy link
Contributor Author

I'm a little new to git myself, the squash instructions fail for me. I'll keep trying stuff if squashing commits is requirement.

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

I'm a little new to git myself, the squash instructions fail for me. I'll keep trying stuff if squashing commits is requirement.

It kinda is because no need to many commit msgs when it only relates to one pkg.

@astralchan
Copy link
Contributor Author

I did git rebase -i HEAD~5 and it pulls up what looks like vim with this

  1 pick f42d507647 qemu: fix patch
  2 pick dba1c48a43 Add tspreed
  3 pick b127dd90eb New package: tspreed-1.2.1
  4 pick d7e25a2f92 polybar: update to 3.5.1.
  5 pick e945ff7070 amfora: install .desktop, sample config
  6 pick ee18ba1f2f vapoursynth: update to R52
  7 pick f802014c3e Removed vlicense line
  8
  9 # Rebase 079bba41c1..f802014c3e onto 079bba41c1 (7 commands)
 10 #
 11 # Commands:
 12 # p, pick <commit> = use commit
 13 # r, reword <commit> = use commit, but edit the commit message
 14 # e, edit <commit> = use commit, but stop for amending
 15 # s, squash <commit> = use commit, but meld into previous commit
 16 # f, fixup <commit> = like "squash", but discard this commit's log message
 17 # x, exec <command> = run command (the rest of the line) using shell
 18 # b, break = stop here (continue rebase later with 'git rebase --continue')
 19 # d, drop <commit> = remove commit
 20 # l, label <label> = label current HEAD with a name
 21 # t, reset <label> = reset HEAD to a label
 22 # m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]
 23 # .       create a merge commit using the original merge commit's
 24 # .       message (or the oneline, if no original merge commit was
 25 # .       specified). Use -c <commit> to reword the commit message.
 26 #
 27 # These lines can be re-ordered; they are executed from top to bottom.
 28 #
 29 # If you remove a line here THAT COMMIT WILL BE LOST.
 30 #
 31 # However, if you remove everything, the rebase will be aborted.
 32 #

@ericonr
Copy link
Member

ericonr commented Dec 12, 2020

You want to make it look like:

  1 pick f42d507647 qemu: fix patch
   4 pick d7e25a2f92 polybar: update to 3.5.1.
  5 pick e945ff7070 amfora: install .desktop, sample config
  6 pick ee18ba1f2f vapoursynth: update to R52
2 pick dba1c48a43 Add tspreed
  3 squash b127dd90eb New package: tspreed-1.2.1
  7 fixup f802014c3e Removed vlicense line

Then edit the final commit message

@ericonr ericonr closed this Dec 12, 2020
@ericonr ericonr reopened this Dec 12, 2020
@ericonr
Copy link
Member

ericonr commented Dec 12, 2020

Wow! Really sorry, I must have misclicked

@astralchan
Copy link
Contributor Author

astralchan commented Dec 12, 2020

[amber@otaku ~/projx/repos/void-packages]$ git push origin tspreed
Username for 'https://github.com': kawaiiamber
Password for 'https://kawaiiamber@github.com':
To https://github.com/kawaiiamber/void-packages.git
 ! [rejected]              tspreed -> tspreed (non-fast-forward)
error: failed to push some refs to 'https://github.com/kawaiiamber/void-packages.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

This was after I did git rebase -i HEAD~5 and made it look like suggested with squash and fixup. Then, git rebase --continue, then the above...

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

You have to force push,: git push origin tspreed -f

@astralchan
Copy link
Contributor Author

Force pushed squashed commits.

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

You didnt edit the first one to change commit message and now something happened to break lint again

Edit: Seems change of #template line got reverted

@astralchan
Copy link
Contributor Author

I think everything is fixed now. Thanks for the help.

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

Almost but no. You added to commit message, not changed change p to r in git rebase -i HEAD~1

@astralchan
Copy link
Contributor Author

Oh. How would I fix it? I take it I first to git rebase -i HEAD~1, then what exactly does the file need to look like, then the next prompt?

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

Since you added, Remove the first two lines and write: New package: tspreed-1.2.1

@astralchan
Copy link
Contributor Author

Upon running git rebase -i HEAD~1, is this what it should look like?

  1 pick 1cfc9e3113 New package: tspreed-1.2.1
  2
  3 # Rebase ee18ba1f2f..1cfc9e3113 onto ee18ba1f2f (1 command)
  4 #
  5 # Commands:
  6 # p, pick <commit> = use commit
  7 # r, reword <commit> = use commit, but edit the commit message
  8 # e, edit <commit> = use commit, but stop for amending
  9 # s, squash <commit> = use commit, but meld into previous commit
 10 # f, fixup <commit> = like "squash", but discard this commit's log message
 11 # x, exec <command> = run command (the rest of the line) using shell
 12 # b, break = stop here (continue rebase later with 'git rebase --continue')
 13 # d, drop <commit> = remove commit
 14 # l, label <label> = label current HEAD with a name
 15 # t, reset <label> = reset HEAD to a label
 16 # m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]
 17 # .       create a merge commit using the original merge commit's
 18 # .       message (or the oneline, if no original merge commit was
 19 # .       specified). Use -c <commit> to reword the commit message.
 20 #
 21 # These lines can be re-ordered; they are executed from top to bottom.
 22 #
 23 # If you remove a line here THAT COMMIT WILL BE LOST.
 24 #
 25 # However, if you remove everything, the rebase will be aborted.
 26 #

@ndowens
Copy link
Contributor

ndowens commented Dec 12, 2020

Yup

@n-ivkovic
Copy link

The changes merged in n-ivkovic/tspreed#8 have now been merged into master and the v1.2.2 tag + release have been moved to include these Makefile changes.

@paper42
Copy link
Member

paper42 commented Dec 22, 2020

... and the v1.2.2 tag + release have been moved to include these Makefile changes.

@n-ivkovic Thanks, however moving a tag is generally not a good thing to do. Why? For example this PR was broken until @kawaiiamber fixed it. If you wanted to build it, it would fail, because the checksum changed. Also, you broke the AUR package where it's a bigger a problem, because users build the software manually there, so a checksum mismatch will (I think) jump at them them every time they try to install tspreed. If you want to add something new, just make a new release.

@astralchan
Copy link
Contributor Author

Removed do_install() and added build_style=gnu-makefile, as well as updated the checksum.

@n-ivkovic
Copy link

...moving a tag is generally not a good thing to do. Why? For example this PR was broken until kawaiiamber fixed it. If you wanted to build it, it would fail, because the checksum changed. Also, you broke the AUR package where it's a bigger a problem, because users build the software manually there, so a checksum mismatch will (I think) jump at them them every time they try to install tspreed

Apologies for this not-so-great approach. I was too focused on finding a quick and easy solution to including the updated Makefile without incrementing the version number (since there was no change to the actual script) and/or without breaking semantic versioning and did not think too far ahead about the consequences. I'll be sure to avoid moving tags unless absolutely necessary in future.

@astralchan astralchan force-pushed the tspreed branch 7 times, most recently from d2e34a7 to d0551ae Compare January 13, 2021 10:15
@astralchan
Copy link
Contributor Author

I've extensively tested the tspreed template with xbps-src and xbps-install to install locally. It works great with no bugs.

@n-ivkovic
Copy link

n-ivkovic commented Mar 25, 2021

Hi @kawaiiamber, tspreed has now been updated to v2.0.0. If you wish to update this PR it should be a simple version+revision and checksum update. Thank you!

@astralchan astralchan changed the title New package: tspreed-1.2.2 New package: tspreed-2.0.0 Mar 26, 2021
@astralchan
Copy link
Contributor Author

Hi @kawaiiamber, tspreed has now been updated to v2.0.0. If you wish to update this PR it should be a simple version+revision and checksum update. Thank you!

Updated the template to 2.0.0

@astralchan
Copy link
Contributor Author

I guess strictly speaking, this new script isn't POSIX, but most shells should have one of the two functionalities needed (if not both) to make the script work.

@astralchan astralchan changed the title New package: tspreed-2.0.0 New package: tspreed-2.0.1 Mar 30, 2021
@astralchan astralchan force-pushed the tspreed branch 2 times, most recently from af3f2df to 9e5991a Compare April 3, 2021 20:49
@astralchan
Copy link
Contributor Author

astralchan commented Apr 3, 2021

Since tspreed is a shell script, it does not build. So I just had the do_build phase return true to skip it. Otherwise, the "help" rule for the upstream makefile displays.

@ericonr
Copy link
Member

ericonr commented Apr 18, 2021

While interesting, this application is just a shell script, which means it doesn't fit within our quality requirements for becoming a package.

Closing for now, sorry.

@ericonr ericonr closed this Apr 18, 2021
@astralchan
Copy link
Contributor Author

astralchan commented Apr 18, 2021

While interesting, this application is just a shell script, which means it doesn't fit within our quality requirements for becoming a package.

Closing for now, sorry.

Quality Requirements

To be included in the Void repository, software must meet at least one of the following requirements. Exceptions to the list are possible, and might be accepted, but are extremely unlikely. If you believe you have an exception, start a PR and make an argument for why that particular piece of software, while not meeting any of the following requirements, is a good candidate for the Void packages system.

System: The software should be installed system-wide, not per-user.

Compiled: The software needs to be compiled before being used, even if it is software that is not needed by the whole system.

Required: Another package either within the repository or pending inclusion requires the package.

In particular, new themes are highly unlikely to be accepted. Simple shell scripts are unlikely to be accepted unless they provide considerable value to a broad user base. New fonts may be accepted if they provide value beyond aesthetics (e.g. they contain glyphs for a script missing in already packaged fonts).

tspreed is installed system wide, not just for the user. So one of the quality requirements is met. Furthermore:

If you believe you have an exception, start a PR and make an argument for why that particular piece of software, while not meeting any of the following requirements, is a good candidate for the Void packages system.
Simple shell scripts are unlikely to be accepted unless they provide considerable value to a broad user base.

I'd make an argument that this shell script "provide[s] considerable value to a broad user base." There are other packages that are just shell scripts.

I would argue this PR should be re-opened on the grounds that:

  1. It meets at least one quality requirement as it is installed system-wide
  2. The shell script "provide[s] considerable value to a broad user base."

@ericonr

@sgn
Copy link
Member

sgn commented Apr 18, 2021

System: The software should be installed system-wide, not per-user.
tspreed is installed system wide, not just for the user. So one of the quality requirements is met. Furthermore:

Well anything could be installed system-wide, but I don't think this package should be installed system wide.

  1. The shell script "provide[s] considerable value to a broad user base."

I don't think this package has broad user base. It seems like only you interested in it. AUR has only 2 votes.

@astralchan astralchan deleted the tspreed branch April 20, 2021 00:31
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-package This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants