-
-
Notifications
You must be signed in to change notification settings - Fork 76
Guidelines partial review #246
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
Conversation
and use a proper noun for an entity.
unstructured and hard to find specific guidelines when scattered. Add some rule guideline based from the general section. Shuffle related paragraphs under relevant sections.
GitHub has an annoying habit that even if you don't want to merge all pull requests it will anyway if in the same sequence of commits. But that seems different and I'd guess one could not just merge only some commits of a pull request and not others, especially if they seem to merge all pull requests even if you don't want them to. So probably not. |
|
Unless you did something funny this is wrong? -# Guidelines for [Rule 12 - UTF-8](rules.html#rule12-utf8)
+# Guidelines for [Rule 12 - UTF-8](rules.html#rule-12---utf-8)The link is what I had it as - unless you added it back or you rely on it being automatically generated (but the one I have it as is easier to type too). |
|
Well I consider separate branches for each commit BUT some changes involve previous ones. As for the last I support I could create a new branch without if @lcn2 considers it too drastic and he just cancels/closes this without merging with comment. |
Not complaining of course. Just saying that GitHub is funny and it seems unlikely it would work either way. |
BOTH work, because NOW there are almost three anchors per rule, which is confusing and lots of work to maintain. |
So I thought but why change it then? I mean fine but isn't it just extra work? Not complaining here either - genuinely wonder why you wanted to do it when it worked. |
|
SIDEBAR: I'm not pleased by the long Rule title names. Makes it harder to remember when you want to type in the anchor manually. Preferred the mnemonic versions. Oh well. |
.. and some of them did not make sense was the problem. And that's also why I added anchors so you don't have to type in longer ones. .. of course you could add the other ones back if you wanted. |
Simple confusion on my par as I was moving about the document. The title anchor always exists, while some of the extra anchors do not (I suppose I could have use |
I really tired of this topic. Its done, dusted, @lcn2 made a choice. |
Fair enough. You know I almost did that and might have in one case. Was just wondering. |
It didn't make sense to me in the slightest fwiw but I'll drop it since you seem ill-disposed to continuing it. |
lcn2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @SirWumpus there are a number of good edits to this PR. Thank you for your time.
We will manually process this PR, with perhaps a slight commit mod (only if needed).
|
I forgot to change one...
to
|
Apologies for the 4th one. I got carried away and should have done it in smaller units. Do you want me to continue afterwards? |
I thought it was 'done' so I didn't do anything. It turns out that was a good thing. If you want I can try taking another look (say for typos) but I should guess that might be too late given it's going to enter pending status in under two days from now. I might not even have the energy to do it and besides it's not like it has to be perfect. There is no perfection anyway. |
Wish you'd wait till I finish the remaining review I'd like to do. The last 1/3 or 1/4 of teh doc. |
|
There are warnings in |
Yes all the more reason it likely is too late. I certainly can't do it today and then it's probably too late tomorrow. But that's what I meant by it turns out it was good I did not look at it. |
Sorry. I might have rushed a little after lunch when I saw you pushed more commits that I needed to merge. |
What I find interesting is the warning not a fatal error. When I forgot to add a closing |
It likes me better 😛 |
|
We will hold off on any changes and PR processing .. sorry we thought you were finished. We will WAIT until you say your PR is ready. You might wish to consider this patch, @SirWumpus For the record, here is what we would have committed on top if the PR when we looked at it: UPDATE 0cOur paste into a GitHub comment was botched. Here is the patch we suggest that you consider: We like the direction you are going in, @SirWumpus, please continue. We will now back out and wait until you give us an "all clear" comment .. or something obvious to that effect. 😉 |
|
p.s. @SirWumpus Some you might be planning on anyway: For the new guidelines sections that relate to rules, link to them in You might be planning that anyway, if not, consider doing that in a patch to this PR. |
Will apply patch. Are what I have present thus far more or less acceptable (I know there were some not so much)? |
We like the direction you are going in, @SirWumpus, please continue. See also GH issuecomment-3598932425. |
Further Reading block.
|
Ignore the "self-requested a review" @SirWumpus, if you please. That is just GitHub noting that when you finish this PR, we will review it again. |
|
@lcn2 I've come to a junction with the bulk of changes. I left untouched and skipped I glanced over the Anyway I think this is good junction to review and integrate this PR.
|
|
As for chongo: do you know the story how @lcn2 got the login? It's a fun one. |
I asked that question on Discord #ask-a-judge about 40 minutes ago. |
Quick updateWe are working on an minor patch to this PR ... |
Fixed links between rules and guidelines. Add guideline about "Rule 2 - Size restrictions" not changing soon. Expand "Guidelines for Rule 13 - No carriage returns in prog.c". Put the word, "restrictions" into "Rule 2 - Size restrictions". While this is a **GOOD IDEA**, the concept needs to be rolled out over all of the rules, not just "Rule 2 - Size restrictions". Changed IOCCC Rules to version **29.13 2025-12-01**. Changed IOCCC Guidelines to version **29.06 2025-12-01**. Ran `make quick_www` to perform the above.
|
With commit 8a95f0c we have modified this PR .. checking the result .. |
Simplify link tags in rules and guidelines. Ran `make quick_www` to perform the above.
lcn2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With commit 8a95f0c and commit a3fb61a this PR is ready.
Thank you 🙏 for your work on this PR, @SirWumpus
This is a partial review with 4 commits in the branch, the last being the most complex. Not sure PRs like this if your can cherry-pick those you approve of.
The most notable commit :
If you approve of this direction I'll continue; not much more remains, but needed a break (and see where this stands).