Generated file inconsistency

[RobinDaugherty]

Btw, I sent this as a message to the recommended Google Group, but it was rejected apparently because I'm not in the group. Might be best to remove it from Github issue template.

When I ran bin/generate-all.sh several days ago on the main branch, it changed commands/types/dnscontrol.d.ts, changing the word "reuse" to "re-use".

There was no change to SPF_BUILDER.md, the source of this line. In fact it hadn't changed in 9 years. It said "re-use" the whole time. https://github.com/StackExchange/dnscontrol/blame/751ae63a7116d8add81c51904d7e298903a46621/documentation/language-reference/domain-modifiers/SPF_BUILDER.md#L288

Someone else (in an unrelated PR) committed the word "re-use" to the generated file: https://github.com/StackExchange/dnscontrol/pull/3958/changes#diff-b793e6dd0f121810d8c490c608a57c26a164d48965556339ce4f0c92c0fb70d3

But then a different PR modified SPF_BUILDER.md to change the word to "reuse": https://github.com/StackExchange/dnscontrol/pull/3947/changes#diff-ca61249407f54d6717b5063b7d535b8bc8bcaa49a05de5456a7313447ffe6ac6

I also noticed something strange: this file is in .gitignore. (However, the file is not actually ignored by git for me (and I'm not sure why). Perhaps it is for some?)

It was added to .gitignore and removed from the repo because it's generated by the release process. #2704

But since this file is generated from the changes made in each PR, I suggest the generator should be run as part of CI to ensure it doesn't break unexpectedly, and the modifications should be committed by the developer that causes the generated file to change. Otherwise it would only fail at release time. And apparently I'm not the only one: the file was added back to the repo not long after it was removed: #2712

I'm happy to work on a fix if there's agreement on a direction.

I suggest:

• CI job to run generate-all and ensure that there are no uncommitted changes in the generated files.
  • Perhaps this should be added to an existing PR workflow, but currently the workflows are named for their one and only job.
• Remove /commands/types/dnscontrol.d.ts from .gitignore

[chicks-net]

I was also concerned about why this change happened in my PR. #3958 (comment). I'm not sure where the dash came from.

[tlimoncelli]

Ah, I think I found the problem!

The reuse/re-use issue came from #3940 which updated the file directly instead of updating documentation/language-reference/domain-modifiers/SPF_BUILDER.md and letting bin/generate.sh do its job. I didn't notice that during review. My bad.

After that there were a series of merges, some of which may or may not have run bin/generate.sh. I let them slide because I always run bin/generate.sh right before a new release. (Should I be more strict?)

I'm happy to work on a fix if there's agreement on a direction.

I suggest:

• CI job to run generate-all and ensure that there are no uncommitted changes in the generated files.

  • Perhaps this should be added to an existing PR workflow, but currently the workflows are named for their one and only job.

@RobinDaugherty: I agree. This is already in .github/workflows/pr_check_git_status.yml but it has gotten out of sync with bin/generate.sh. I'd appreciate it if you could get them in sync again. Heck, there's no reason we don't just run bin/generate.sh.

• Remove /commands/types/dnscontrol.d.ts from .gitignore

Agreed.

Best,
Tom

[chicks-net]

get them in sync again

I've had really good luck using just as a framework for this sort of thing. https://just.systems/

For situations like this I have the github action call a just recipe to do the generation or testing. It is still available to run locally and it doesn't leave another script file laying around in the repo.

You can check out https://github.com/fini-net/template-repo/tree/main/.just to see how far I've pushed this for my personal projects. I'd be happy to do a PR with the justfile I've been using for dnscontrol development.