Skip to content
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

Use baseline mtime instead of current time #188

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mwikberg-virta
Copy link

@mwikberg-virta mwikberg-virta commented Dec 5, 2024

Added flag --mtime to use the baseline file modification timestamp as the date instead of "now", because it makes much more sense when you need to generate multiple reports from the same repository or want to track actual changes to the baseline

@staabm
Copy link
Owner

staabm commented Dec 5, 2024

Which problem should this PR solve? Would #91 also work for you?

@mwikberg-virta
Copy link
Author

The problem is that all JSON files got the same (current) date, so the graph is not very useful

[
  {
    "phpstan-baseline.neon": {
      "Date": "Fri, 29 Nov 2024 17:01:16 +0000",
      "Overall-Errors": 4062,
      "Classes-Cognitive-Complexity": 0,
      "Deprecations": 44,
      "Invalid-Phpdocs": 37,
      "Unknown-Types": 3,
      "Anonymous-Variables": 0,
      "Native-Property-Type-Coverage": 0,
      "Native-Param-Type-Coverage": 0,
      "Native-Return-Type-Coverage": 0,
      "Unused-Symbols": 0
    }
  }
]

image

versus

image

with the correct dates..

The ksort was also needed to make sure the data is displayed correctly.. Otherwise data points were rendered in some very weird random order, which makes no sense for a graph that should have time as one axis

@mwikberg-virta
Copy link
Author

At least I guess that the actual modification time of the baseline is more correct than "now" in most cases? It could maybe be optional though...

@staabm
Copy link
Owner

staabm commented Dec 14, 2024

did you check whether the fix from #91 would work for you?
(without the modifications of this PRs here)

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