tdrhq / slite

a SLIme-based TEst runner for FiveAM and Parachute Tests
Apache License 2.0
50 stars 3 forks source link

Add library header, address compiler warnings, and some cleanup #7

Closed tarsius closed 1 year ago

tarsius commented 1 year ago

This is a follow up to https://github.com/plandes/flex-compile/commit/c6f7b97e96b4f2608954f8f599d255578c73d600.

Please open the Melpa pull-request yourself.

/cc @plandes

tdrhq commented 1 year ago

Thanks for the PR, let me quickly give it a test. Does this have all the changes required for me to open a PR on Melpa?

tdrhq commented 1 year ago

Merged! Just a heads up the merge didn't go through GitHub so this will show as Rejected, but the merge has been done.

tarsius commented 1 year ago

Cheers!

plandes commented 1 year ago

@tdrhq I strongly recommend adding the keybindings as a minor mode (preferred) or a function that the user can call. Changing global keybindings is bad because many of those are rebound by the user (my situation as well). But it appears from the latest you didn't clobber the global keybindings.

However, question, what is the dfdfdf on line 58?

tdrhq commented 1 year ago

@plandes lol, that's my bad attempt at a docstring :)

tarsius commented 1 year ago

Just a friendly reminder: the next step is to open a pull-request to add this to Melpa. The package will then be reviewed and it is likely that you would be asked to make some changes to fix issues reported by various linters. I could open the pull-request for you, but am not doing so because the real work is fixing the issues, and you have to be willing to do that, otherwise the pull-request would just be another one that goes nowhere.

plandes commented 1 year ago

@tdrhq Do you have the bandwidth for this? If not, you might consider the transferring the maintainer role to someone else (in the spirt of Eric Raymond).

tdrhq commented 1 year ago

I just finished the checkdoc, and damn it does take a lot of effort, compared to quicklisp.

Looking at the checklist, I've got these three to go, but I doubt I can get to these today:

@plandes Are you offering to maintain? I'd be happy if that's the case. I generally also consider myself to be very responsive to PRs, so if you do send me a PR for these items on the checklist, I'll probably merge them in more or less blindly. :)

plandes commented 1 year ago

@tdrhq Re maintain: absolutely not, I do not have time :). It was a helpful suggestion--at least in my own thinking. How helpful it is I suppose is subjective. Haha! Oh and yes, I've been through these things before with my own projects and it is very time consuming. I now run these tools more often as I write to "nip them in the bud".