Skip to content

Conversation

@andig
Copy link
Contributor

@andig andig commented May 3, 2024

Fix #87. Replaces #86. Refs #80 (comment).

Re-exposes

Printf(format string, v ...any)

as minimal logger interface, this time as public type.

Please squash before committing.

/cc @tretmar @srebhan

@andig andig mentioned this pull request May 3, 2024
Copy link
Contributor

@dammarco dammarco left a comment

Choose a reason for hiding this comment

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

We (@hsteidl ) looked into it and it looks good to us.

As we are referencing to a specific commit of this repo, we didn't experience any breaking changes.

But if you require this interface and the library anyway only sends debug log messages, then we are fine with doing this ;)

@dammarco dammarco merged commit 582f2ab into grid-x:master May 3, 2024
@andig
Copy link
Contributor Author

andig commented May 3, 2024

Please squash before committing.

@tretmar didn't work. Makes for ugly commit history and potentially harder to merge in forks.

@dammarco
Copy link
Contributor

dammarco commented May 6, 2024

@andig Just to understand you here... you want to have 1 commit per merged PR in the end?

@andig
Copy link
Contributor Author

andig commented May 6, 2024

I think that would be great. At least when the PR does not contain an intentional logical breakdown of the changes.

@dammarco
Copy link
Contributor

dammarco commented May 7, 2024

Okay, we're working internally with a different approach - so that's why we don't squash before merging... simply because we are used to a different approach ;) We follow the approach that the authors of PRs are responsible to have a clean commit structure, including squashing. Still, in a public repository like this here, it might not be the best approach as we can't force everyone to follow this clean commit approach 🤔

@srebhan
Copy link
Contributor

srebhan commented May 7, 2024

@tretmar well you can force people, but why if you can just squash the commits automatically? :-) We are doing this in Telegraf and you can configure Github to do so.

The big advantage of squashing IMO is that you do have a cleaner commit history keeping things together, but that's a matter of taste of course...

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.

#80 breaks compatibility

3 participants