search

ioccc-src / mkiocccentry

Form an IOCCC submission as a compressed tarball file
Other
30 stars 6 forks source link

Enhancement: Remove first_rule_is_all from .info.json for IOCCC29 #1222

Open lcn2 opened 1 month ago

lcn2 commented 1 month ago

Is there an existing issue for this?

Describe the enhancement

The code to test if all is the first rule in a Makefile is problematic at best.

Relevant images, screenshots or other files

Any current IOCCC28 .info.json file.

Relevant links

Issue #1221

Anything else?

For IOCCC29 (and later contests):

xexyl commented 1 month ago

Already did it .. have to test of course but should be good.

lcn2 commented 1 month ago

Already did it .. have to test of course but should be good.

Did what "it"?

The .info.json version cannot be changed for IOCCC28. Moreover this is an IOCCC29 matter.

xexyl commented 1 month ago

Already did it .. have to test of course but should be good.

Did what "it"?

The .info.json version cannot be changed for IOCCC28. Moreover this is an IOCCC29 matter.

I had thought you said this should be done now. I can certainly undo this although unfortunately that means more juggling with the changes I have made.

xexyl commented 1 month ago

And yes I know it says IOCCC29 here - I had already finished it before you opened this issue as it only took a minute.

lcn2 commented 1 month ago

Already did it .. have to test of course but should be good.

Did what "it"? The .info.json version cannot be changed for IOCCC28. Moreover this is an IOCCC29 matter.

I had thought you said this should be done now. I can certainly undo this although unfortunately that means more juggling with the changes I have made.

Changing the format of .info.json is a HUGE change ‼️

One most certainly should NOT modify the format of a fundamental file such as .info.json while the IOCC28 is open ‼️

Just force the value of first_rule_is_all to be true in .info.json for IOCCC28 only. Let the rest of the code base think that all is well with a true value for first_rule_is_all in the existing .info.json files for IOCCC28 only.

Let this issue, which is explicitly marked as IOCCC29 do the modification of the .info.json format by removing first_rule_is_all from .info.json, updating the IOCCC_info_version, and removing/modifying as needed, code that managed the first_rule_is_all values.

xexyl commented 1 month ago

Already did it .. have to test of course but should be good.

Did what "it"? The .info.json version cannot be changed for IOCCC28. Moreover this is an IOCCC29 matter.

I had thought you said this should be done now. I can certainly undo this although unfortunately that means more juggling with the changes I have made.

Changing the format of .info.json is a HUGE change ‼️

One most certainly should NOT modify the format of a fundamental file such as .info.json while the IOCC28 is open ‼️

Just force the value of first_rule_is_all to be true in .info.json. Let the rest of the code base think that all is well with a true value for first_rule_is_all in the existing .info.json files for IOCCC28.

Let this issue, which is explicitly marked as IOCCC29 do the modification of the .info.json format by removing first_rule_is_all from .info.json, updating the IOCCC_info_version, and removing/modifying as needed, code that managed the first_rule_is_all values.

Sorry. I was thinking of the just setting it to true. This issue has not been resolved. I updated another comment about that but I forgot to update this one. Sorry about that.

lcn2 commented 1 month ago

Sorry. I was thinking of the just setting it to true. This issue has not been resolved. I updated another comment about that but I forgot to update this one. Sorry about that.

Thank you, @xexyl for clarifying 👍

xexyl commented 1 month ago

Sorry. I was thinking of the just setting it to true. This issue has not been resolved. I updated another comment about that but I forgot to update this one. Sorry about that.

Thank you, @xexyl for clarifying 👍

Welcome. Trying to get so much done in so little time can certainly cause confusion - and that can also be from not reading comments carefully.

lcn2 commented 1 month ago

After further reflection (as well as explaining by folk), this issue as been set as a ((TOP PRIORITY)) and is to be included in the pending new releases of this repo.

See also GH-issuecomment-2708487545.

If any changes are needed to the IOCCC guidelines or IOCCC FAQ in "the other repo" as a result of resolving this issue, please issue a PR in "the other repo" to do that.

xexyl commented 1 month ago

After further reflection (as well as explaining by folk), this issue as been set as a ((TOP PRIORITY)) and is to be included in the pending new releases of this repo.

See also GH-issuecomment-2708487545.

If any changes are needed to the IOCCC guidelines or IOCCC FAQ in "the other repo" as a result of resolving this issue, please issue a PR in "the other repo" to do that.

Well as for this one: it might be a mistake to have it removed from the file right now as that does require significant changes in code and some of it is complicated (in particular t he patching system). However I do have the change where it simply sets the value to true and the test function also returns true so that nothing is flagged.

lcn2 commented 1 month ago

We believe we have addressed all of the current questions that still need answering at this time. If we've missed something or something else needs to be clarified, please ask again.

lcn2 commented 4 weeks ago

Important checkpoint

Please see GH-issuecomment-2762004417 for a recent comment on existing issues, priorities, and on a deadline for going into pending state for IOCCC29.