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

License dialog: Always use date handler #6138

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

lukasbestle
Copy link
Member

@lukasbestle lukasbestle commented Jan 12, 2024

This PR …

Fixes

Ready?

  • Unit tests for fixed bug/feature
  • In-code documentation (wherever needed)
  • Tests and checks all pass

For review team

@lukasbestle lukasbestle added the type: bug 🐛 Is a bug; fixes a bug label Jan 12, 2024
@lukasbestle lukasbestle added this to the 4.0.4 milestone Jan 12, 2024
@lukasbestle lukasbestle requested a review from a team January 12, 2024 20:24
@lukasbestle lukasbestle self-assigned this Jan 12, 2024
Copy link
Member

@distantnative distantnative left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering whether we really want to add it as parameter to each method or rather define that the License methods will use the date handler (so passing it inside the methods to Str::date.

@lukasbestle
Copy link
Member Author

For our use, yes. But devs might want to hook into this for custom dashboards etc. Allowing a $handler argument is more consistent with the rest of the system.

@bastianallgeier
Copy link
Member

I think it only feels a bit like unnecessary public features first but on the other hand I agree about the consistency.

@distantnative distantnative merged commit 7c6b435 into develop-patch Jan 15, 2024
12 checks passed
@distantnative distantnative deleted the fix/6134-license-date-format branch January 15, 2024 07:42
@distantnative distantnative linked an issue Jan 15, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panel: Kirby license date format wonky (4.0.2)
3 participants