-
-
Notifications
You must be signed in to change notification settings - Fork 545
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
Correct capitalization of 'atbash' to 'Atbash' #2485
base: main
Are you sure you want to change the base?
Conversation
|
Also, if it doesn't cause additional churn for tracks with test generators, the comments in |
So far 'Atbash' is incorrectly capitalized in:
I'll update those too, if needed. I just need to know which ones. Edit: I guess the affine-cipher instructions need to be fixed for sure. |
I think it would be good to update them all to be consistent. |
This test is correctly failing, but not for reasons of this PR. |
That was fixed in #2487 so we just need a rebase. |
@BNAndras You are referring to this PR? It doesn't seem to be conflicting with the main branch. Only the link check fails, which doesn't affect anything. |
It is not that it is conflicting, it is that it out of date to what it is based on, not allowing the CI tests to pass. If you check out main, get that branch up to date locally, and then rebase this branch to that, and push it up, you will find that the updated changes are in place allowing the CI test to pass. The commands that I would do would be something like this: git checkout main
git pull upstream main
git checkout atbash-cipher
git rebase upstream//main
git push origin atbash-cipher Provided, of course, that the remote for the exercism repository is You may need to force push, which is fine, as this is your own branch, not the history on main on the "canonical repository". To expand, in the web view on your own repository, you will see information such as: "This branch is 2 commits ahead of, 8 commits behind exercism/problem-specifications:main." This is what is causing the problem, there may be some kind of on platform "rebase this to the upstream remote" or something so that you may not need to do this in the command line, but I most often work there, so am more familiar with what needs to be done locally. |
If you want a few less keystrokes, you can also do merge some of those lines.
|
I understand that the CI check links does not pass, what I don't understand why they need to pass? For example, commits on 9 Oct, 22 Oct, 24 Oct, were all having the same issue with the failing links. |
There was a known issue with the check. People were discussing it and trying to figure out how to beat deal with it. While there was a known issue with the check, it was reasonable to ignore the failures. The issue has since been fixed and the check should now be working. There is no longer any reason for the tests to be failing. |
@tasxatzial looks like your fork of the repo is a few commits behind so it didn't get the fux that Isaac was talking about. (It's relatively recent, at least the specific case that's making the CI fail here) |
f8385dc
to
510c20c
Compare
Ok, so no particular reason other than making the checks pass. When BNAndras said it was needed, I thought there was a more serious issue, like deployment failing or something similar, hence the question. Rebased. |
Thanks. I was just clarifying what needed to be after KOTP highlighted the failing test. The PR itself can’t be merged without approval from the code owner. |
Know that the shorter way will not update your main, though, but will only rebase the branch to upstream/main. Probably not a problem if you are aware, but may cause confusion if you are not. |
Also, thanks @tasxatzial for the rebase, and @BNAndras for the clarification, @iHiD for the lesson in git efficiency, and @Cool-Katt for reiteratig and @IsaacG for the time travel exploration! |
@exercism/maintainers-admin, can we get an approval? Please and thank you. |
No description provided.