Skip to content

fix: invert addTimeStamp config logic#130

Closed
oojacoboo wants to merge 1 commit intoperplorm:mainfrom
oojacoboo:fix/add-time-stamp
Closed

fix: invert addTimeStamp config logic#130
oojacoboo wants to merge 1 commit intoperplorm:mainfrom
oojacoboo:fix/add-time-stamp

Conversation

@oojacoboo
Copy link
Copy Markdown

Summary

  • The generator.objectModel.addTimeStamp config option had inverted logic: setting it to true suppressed timestamps, while false (the default) included them.
  • This flips the condition in generateTimestampBlock() so the config behaves as its name suggests — true adds timestamps, false omits them.

Note: the baseQueryClassHeader.php template already had the correct logic (if ($addTimestamp) → show timestamp). This fix brings AbstractOMBuilder in line with the template.

Impact

Projects using the default addTimeStamp: false will no longer get timestamps in generated class headers, eliminating unnecessary noise in version control diffs.

The condition was inverted — `addTimeStamp: true` suppressed
timestamps and `false` (the default) included them. This flips the
condition so the config behaves as expected.
@mringler
Copy link
Copy Markdown
Collaborator

I am confused.

This was already fixed in c08d88f (reference files don't work when timestamps are generated).
And indeed it seems to be the same as the current state in main: https://github.com/perplorm/perpl/blob/main/src/Propel/Generator/Builder/Om/AbstractOMBuilder.php#L618

Why is this showing up as changing something? Am I stupid this morning or what?

@oojacoboo
Copy link
Copy Markdown
Author

Oh, 2 days ago - wow okay. I think the issue is that my fork wasn't synced for the main branch maybe. Not really sure b/c I know I synced it before starting on the other PR. But maybe this fix came after. Closing as already fixed.

@oojacoboo oojacoboo closed this Mar 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants