-
-
Notifications
You must be signed in to change notification settings - Fork 544
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
Proposal: bracket-push: rename to matching-brackets #1501
Conversation
I rehash my prior allegations of #693 below: I contend that having the word `push` in the name unnecessarily biases the solution space toward solutions that use a stack data structure (has a *push* operation) or a *push*-down automaton, rather than other solutions not using either of these two. We have heard a principle that we want to name exercises by their story, not by what they teach: #1451 (comment) The story here is about matching brackets, so I posit that that serves as the name we want. I have changed the major version number, as was done in the retree->satellite rename, even though I do not feel strongly about whether that should be necessary: #1478 As we have gained experience in the rename of retree, we see that problem-specifications is free to rename exercises at any point, without waiting for all tracks to follow suit. Of course, this operation should not be performed lightly since it causes churn in the 31 (*thirty-one*) tracks implementing this exercise.
I have a specific question for @exercism/product : I am proposing here a rename of I would prefer to avoid subjecting 31 tracks (the number of tracks implementing |
Follow-up steps to execute this rename, if this proposal is accepted:
For your consideration, here are the 31 tracks that implement this exercise. Note that the first number you see is the difficulty. The second number you see is the depth of the exercise, defined as follows:
The script that made this is at https://github.com/petertseng/exercism-problem-specifications/blob/list-tracks/implementing-tracks.rb Note that for tracks that have zero |
I agree with your reasoning and am personally content with |
Same here. @iHiD, what do you think? |
Firstly, I'm grateful to @petertseng for such clear reasoning and the offers to do the legwork. As there's three 👍 and no 👎 after a week, I say go for it :) |
I am unsure if we have a policy wrt. merge/rebase commits when there is only one commit in the PR. |
I've never understood the point of merge commits and would appreciate being enlightened :) |
The only use I get out of them is that branches merged with merge commits will show up when On my own personal projects, I tend to use merge commits when there is only 1 commit. |
I would |
I'll describe three common situations we might see in PRs. Merge commits are only required in the very last case (last paragraph) I describe. For this PR which was submitted as one commit and should end up as one commit, it does not seem particularly important, therefore I slightly favour using Squash and merge since the merge commit is not necessary. For PRs which are submitted as multiple commits and should end up as one commit, Squash and merge is a necessity; a merge commit would leave commits in history that are not independently revertable or understandable, and may be in an intermediate state that do not pass tests (rendering the repo no longer bisectable). For PRs which are submitted as multiple commits and should end up as multiple commits, Squash and merge is undesirable, since it creates a single commit that does multiple things, in violation of the principle that each commit should do one thing only. In this case, we must either use Create a merge commit or Rebase and merge. Rebase and merge has the disadvantage that we can no longer tell via |
@petertseng For your third example, a PR with multiple commits that should remain as multiple commits, I would suggest there is a way to tell after a Rebase and merge what is part of a single PR. Have the PR referenced in each commit with the hastag identifier. Since it's a Rebase and Merge, it would all happen at once. That's what my team at work does, we have the TaskID as part of the commit. In out case it's at the beginning, but for this repo it could be Then all the commits reference the exercise or section modified and what the work/change was. |
Recalling that there are 31 tracks that would be affected, one may find it challenging that we don't have any hard numbers/rules on what constitutes sufficient quorum. My personal take on it is: This repo holds a set of recommendations that are suitable for our language-agnostic specifications. Tracks are always free to deviate from them. For this PR, that means they can feel free to choose to ignore this rename and have it remain Assuming the current state of nonzero support + zero objections remains, I plan to merge this PR when I actually have time to perform the additional steps (website-icons PR, issues on implementing tracks). I estimate in 48 hours. |
I don't know the answer to this off the top of my head for certain, so I'll put the question out there: does GitHub not put the PR link in the title for a merge commit? Admittedly, the commits that get merged are not linked to the PR, but there should still be an actual merge commit that links to the PR. |
Nope, unfortunately :) |
You have my 👍 and gratitude on this plan :) |
This rename was proposed in: exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space
This rename was proposed in: exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space
This rename was proposed in: exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space
This rename was proposed in: exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space
@petertseng I took your list and added checkboxes. Checked them if a PR has been opened.
|
This rename was proposed in: exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space
I am quite late, and for this I apologise for missing my obligations, but I filed the issues. |
Rename the bracket-push exercise to matching-brackets. This rename was proposed in: exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space Resolves exercism#1240
* bracket-push: rename to matching-brackets This rename was proposed in: exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space * 👀 rename reference in settings.gradle
As requested in exercism/problem-specifications#1501 and detailed in exercism#253
rename from bracket-push exercism/problem-specifications#1501
rename from bracket-push exercism/problem-specifications#1501
This rename was proposed in exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space
This rename was proposed in: exercism/problem-specifications#1501 The rationale for the name is: * to name the exercise by its story, not by what it potentially teaches * to avoid unnecessarily biasing the solution space
Summary: Here is my official proposal to rename bracket-push, which wants your comments. We have gained experience from the
retree
->satellite
rename that renames are safe to do. Reviewers of this proposal should carefully consider whether the proposed change warrants the costs that will be incurred by the 31 tracks that have implemented this exercise.We can consider discussion on what the new name should be, but if nobody has any other options, I defaulted to
matching-brackets
. Another possibility I thought of isbracket-matching
.I have another comment coming up with necessary follow-up steps that would need to be taken if this proposal is ratified.
Below is the original commit message, which contains rationale for the change.
I rehash my prior allegations of
#693 below:
I contend that having the word
push
in the name unnecessarily biasesthe solution space toward solutions that use a stack data structure (has
a push operation) or a push-down automaton, rather than other
solutions not using either of these two.
We have heard a principle that we want to name exercises by their story,
not by what they teach:
#1451 (comment)
The story here is about matching brackets, so I posit that that serves
as the name we want.
I have changed the major version number, as was done in the
retree->satellite rename, even though I do not feel strongly about
whether that should be necessary:
#1478
As we have gained experience in the rename of retree, we see that
problem-specifications is free to rename exercises at any point, without
waiting for all tracks to follow suit.
Of course, this operation should not be performed lightly since it
causes churn in the 31 (thirty-one) tracks implementing this exercise.