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

Add recipe for dumber-jump #9065

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

Conversation

zenspider
Copy link
Contributor

@zenspider zenspider commented Jun 13, 2024

Brief summary of what the package does

Dumber Jump is a fork from dumb-jump that removes as many
configuration options as possible to go back to the roots of being a
fairly config-free "dumb" jump. This version doesn't support other
searchers, doesn't support alternative display methods, and only
interfaces through xref.

Note: while this package is a "fork", it is a massive divergence at
this point:

$ git diff master.. | lc
    5699

byte-compile is mostly complaining about 80 char widths at this point.

checkdoc... ugh. this tool. working on it.

Direct link to the package repository

https://github.com/zenspider/dumber-jump/tree/dumber-jump

Your association with the package

I am the maintainer (of this fork)

Relevant communications with the upstream package maintainer

None needed

Checklist

@alphapapa
Copy link
Contributor

I think you will need to rename the symbols to dumber-jump as well as update the documentation accordingly. Otherwise you're essentially suggesting that dumb-jump be replaced in MELPA with your fork.

FYI for help ensuring clean compilation, etc, you might try https://github.com/alphapapa/makem.sh (see also its makem.el menu).

@zenspider
Copy link
Contributor Author

@alphapapa I don't think you're looking at the branch I'm publishing: https://github.com/zenspider/dumber-jump/tree/dumber-jump

@alphapapa
Copy link
Contributor

alphapapa commented Jun 14, 2024

@alphapapa I don't think you're looking at the branch I'm publishing: https://github.com/zenspider/dumber-jump/tree/dumber-jump

I'd guess that you should merge that to your master branch, then. Otherwise visitors to your repo will see an old commit from an old fork. It's awkward for the name of the repo to have changed, but the master branch to represent a different repo.

Alternatively, start a new repo that GitHub doesn't consider a fork; it's not necessary for GitHub to recognize it as one, anyway.

@zenspider
Copy link
Contributor Author

I assume visitors visiting via melpa click the github link and it goes to the branch, no?

I'd like to maintain upstream in order to rebase (if they ever commit anything again). Suggestions welcome.

@alphapapa
Copy link
Contributor

I assume visitors visiting via melpa click the github link and it goes to the branch, no?

I can only say, again, that it would be awkward for the URL https://github.com/zenspider/dumber-jump to actually point to a clone of dumb-jump, and for the package that the repository exists to host to be found in a branch other than master. Such an arrangement would be very unusual and likely to confuse users (especially if they were to submit a PR).

I'd like to maintain upstream in order to rebase (if they ever commit anything again). Suggestions welcome.

You needn't have your clone's master branch be a clone of the forked repo's master branch. You needn't have the dumb-jump master branch correspond to any branch that you push to your own GitHub repo. You can simply add the dumb-jump repo as a remote on your own system, track it in a local branch, and pull changes from it as desired.

Note as well that you should not be rebasing your package's master branch (whether it's named master or something else). Doing that would cause conflicts which would force downstream users to continually hard-reset their local branches; that includes any downstream packaging of your package.

The rule of thumb is to never rebase a published master branch, because that rewrites the history of all the commits you've rebased, changing all of their hashes, and causing conflicts down the line (e.g. imagine a user with a PR branch on top of your master branch, and then you rebase your master branch...). Feature branches are fine to rebase, of course, as they develop. This is not specific to MELPA or Emacs packages; it's standard Git workflow.

@riscy
Copy link
Member

riscy commented Jun 16, 2024

I agree with @alphapapa here, and usually "such an arrangement would be very unusual and likely to confuse" is the best litmus test. The default branch shouldn't exist solely to be a fork-point (if I'm understanding correctly).

Here's some quick feedback from the lints, but note that checkdoc is considered within reason these days. Checkdoc does check some crucial stuff, so be sure you check.

dumber-jump.el with byte-compile using Emacs 29.3:

dumber-jump.el:59:2: Warning: custom-declare-variable `dumber-jump-rg-word-boundary' docstring wider than 80 characters
dumber-jump.el:65:2: Warning: custom-declare-variable `dumber-jump-fallback-regex' docstring wider than 80 characters
dumber-jump.el:102:2: Warning: custom-declare-variable `dumber-jump-find-rules' docstring wider than 80 characters
dumber-jump.el:1545:2: Warning: custom-declare-variable `dumber-jump-language-contexts' docstring has wrong usage of unescaped single quotes (use \= or different quoting)
dumber-jump.el:1591:2: Warning: custom-declare-variable `dumber-jump-project' docstring wider than 80 characters
In dumber-jump-message:
dumber-jump.el:1624:2: Warning: docstring wider than 80 characters
In dumber-jump-get-language:
dumber-jump.el:1690:2: Warning: docstring has wrong usage of unescaped single quotes (use \= or different quoting)
In dumber-jump-get-language-from-mode:
dumber-jump.el:1706:2: Warning: docstring wider than 80 characters
dumber-jump.el:1706:2: Warning: docstring has wrong usage of unescaped single quotes (use \= or different quoting)
In dumber-jump-get-results:
dumber-jump.el:1728:2: Warning: docstring wider than 80 characters
In dumber-jump-filter-no-start-comments:
dumber-jump.el:1918:2: Warning: docstring wider than 80 characters
In dumber-jump-process-results:
dumber-jump.el:1936:4: Warning: More than one doc string
In dumber-jump-shell-command-switch:
dumber-jump.el:2042:2: Warning: docstring wider than 80 characters
In dumber-jump-parse-response-line:
dumber-jump.el:2080:2: Warning: docstring wider than 80 characters
In dumber-jump-parse-response-lines:
dumber-jump.el:2105:2: Warning: docstring wider than 80 characters
In dumber-jump-get-contextual-regexes:
dumber-jump.el:2158:2: Warning: docstring wider than 80 characters
In dumber-jump-get-rules-by-language:
dumber-jump.el:2215:52: Warning: Unused lexical argument `searcher'
dumber-jump.el:2231:41: Warning: `point-at-bol' is an obsolete function (as of 29.1); use `line-beginning-position' or `pos-bol' instead.
In end of data:
dumber-jump.el:2020:29: Warning: the function `tramp-dissect-file-name' is not known to be defined.
dumber-jump.el:2019:28: Warning: the function `tramp-file-name-localname' is not known to be defined.

dumber-jump.el with melpazoid:

- The `;;; Commentary` for this file is much wider than 80 characters
- dumber-jump.el#L1627: It's safer to sharp-quote function names; use `#'`

@riscy
Copy link
Member

riscy commented Jun 30, 2024

Friendly ping. :)

@riscy riscy added the awaiting-upstream Awaiting action from an upstream maintainer label Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-upstream Awaiting action from an upstream maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants