Remove NimblePkgList

[juancarlospaco]

Remove NimblePkgList

• Remove NimblePkgList (Dead code).

[alaviss]

Thanks for the contribution, but my complaints on the other PR also apply here: #128 (review)

[saem]

Haxscramper read over the pr and commit guidelines and updated them sustainability in order to bring them more online with the norms we keep within our community: https://nim-works.github.io/nimskull/contributing.html#pr-and-commit-message-guidelines 🎉

Specifically, I think a good structure for the "why" part of the commit should be:

• what it is and does
• how it's inclusion detracts from the direction (sustainability); and other ways it's problematic
  • dead code, can happen outside, can do something much better later, whatever the thought process
• any future considerations or inspiration

The present commit message needs some improvement in order to hit the aforementioned goals. If you have questions about the above I'm happy to help.

[juancarlospaco]

Github/Git has the option to Squash and Merge, why dont just do that ?. 🤔

I do not have time for this, accomodating things to the way you feel is Ok to you,
I think is Ok if you want to do it for yourself, but forcing it into others not so much,
besides thats the reason why Git has a history and tools to analyze the history,
I think that anything that erases commits from existence is not good whatsoever.

[haxscramper]

Assuming you mean we can rewrite git comment when we squash - this approach is hardly sustainable. We cannot infer author's reasoning and understanding for every PR we recieve.

I do not have time for this, accomodating things to the way you feel is Ok to you,

We have written down contribution guidelines specifically for this matter - it is not a question about whether something "feels" ok to us - we agreed to follow that specific set of rules, and we intend to adhere to them. Those are not especially hard to follow - you already have an understanding why this change needs to be made, so it can be written down quite easily.

I think that anything that erases commits from existence is not good whatsoever.

We want to make sure that every commit in the git history passes the full test suite, instead of having a patchwork of red and green ticks on the CI, mixed with "make CI green again" commits. For technical reason, we haven't found an acceptable way to run a full CI test on each commit in the PR, and for now we require all commits to be squashed.

[haxscramper]

I want to reiterate that by no means we want to alienate potential contributors, but there are some minimal requirements to the submissions and those are mandatory. In this specific instance, we are talking about writing several lines in the commit message.

[juancarlospaco]

• what it is

It is dead code.

• how it's inclusion detracts from the direction

I am not including anything.
Just removing dead code.

• any future considerations

No.

I hope it responds your questions.

[saem]

OK, I had to do some spelunking and I'm curious what of the following you knew already or can correct:

• nimblepkglist at one point was used to generate a list of nimble packages as part of Nim website build
• nimskull doesn't support nimble

[saem]

Aside, can you please stop applying 👎🏽 to people's comments. Especially when they've taken the time to patiently explain each thing. It's fine to disagree, but folks aren't providing one word/line responses nor are they not taking the time to review things, it comes off as really dismissive. Thank you.

[juancarlospaco]

stop applying 👎🏽 to people's comments.

Feel free to hide or delete my comments.

Especially when they've taken the time to patiently explain each thing.

Thats what I agree too, lets not waste time in this.

[haxscramper]

Feel free to hide or delete my comments.

That's not how we see meaningful and productive relationship - instead of censoring each other out, we try to talk through and formulate mutual understanding about different things. As I said earlier, we don't want to push anyone away, but there are some rules that were explicitly written down, such as minimal requirements on commit guidelines, and those must be followed.

Thats what I agree too, lets not waste time in this.

I'm pretty sure writing down a proper commit message does not take that much time (especially compared to arguing over the necessity of a couple text lines for almost a week). It seems to me you have already explained your reasoning (and saem provided more elaboration on the context) in one of the previous comments, so something similar to the

Remove NimblePkgList

nimblepkglist at one point was used to generate a list of nimble 
packages as part of Nim website build. Nimskull does not support
nimble, so there is no reason to keep this dead code in the repo

Admittedly, I'm no master of commit messages (and I'm not familiar with code in question, so there might be additional considerations), but this provides at least a minimal level of context that answers questions like "what changed?", "why changed?"

[haxscramper]

While I have no objections to the PR itself, commit messages must conform to the minimal requirements as explained in the contribution guidelines.

[juancarlospaco]

The commit message is already in place, and is clear about the "why",
if you want something else, feel free to just edit the commit message directly,
you can do it from the web browser, Ive done it lots of times in my projects PR.

I really dont understand whats the problem with commits message in this project,
I contrib to other open source projects of all kinds and sizes,
and never been blocked by commit message content bureaucracies.

Just edit it, no problem.

[haxscramper]

With all due respect, I think we have already made it clear that "you can just edit to whatever you feel ok" is not a sustainable approach. Proper commit messages are a part of the procedure for merging PRs. We have already provided a missing elaboration with example.

And more on the "you can just ..." - yes, we can edit commit messages to whatever we think is appropriate, let in any commit messages and then try to infer the author's reasoning behind all the changes, piece together the whole reasoning or try to come up with some "if I were doing this thing, why I would've done this?". In this specific instance, it just so happened that both #130 and this PR have 'sort of' fitting context - we can just copy-paste things from somewhere. So this one might feel like overly bureaucratic where we pointlessly waste time when we could've just edited things ourselves.

We wouldn't have this discussion if there was a failing test, or there was some absolutely illegible chunk of code, right? We also could've "just" fixed them, technically this is not a problem, right? We can try to solo this entire project, but that is not sustainable. We specifically spent time to actually explain, at length, what exactly do we find wrong in this commit message, provide the example and elaborate why this addition is necessary. We want to make sure that every contributor understands that some parts of this project are specifically important, instead of handwaving a commit message and then letting us fix up after them. Just as you seemingly still don't understand the commit message "problem" here, it really puzzles me why, even after all the provided examples and suggestions it still takes two weeks to make another attempt, and it is wasted on further extension of the debate. Just like we can "Just edit it", I don't think it would be a problem for you to run the minimal checklist and/or call git commit --amend and edit in a couple of missing lines.

I'm talking for both open PRs here - #130 also included. Although I think there is no question that "Remove DrNim" and "this is just stupid" barely qualify for the proper commit messages.

And if you want to talk about this part in faster context, we can talk this on matrix channel you have already joined, I think it would be quicker than spending three weeks on the slow back and forth.