Skip to content

Conversation

@winterqt
Copy link
Contributor

Closes #148.

Copy link
Member

@zombiezen zombiezen left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM after one small comment. We should also document this function: mind sending a PR over to https://github.com/256lights/zb-docs/blob/main/lua/extensions.md ?

Copy link
Member

@zombiezen zombiezen left a comment

Choose a reason for hiding this comment

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

I overlooked something, sorry:

@winterqt
Copy link
Contributor Author

No clue what’s up with that test failure; flaky test (as it passes on my machine)?

@zombiezen
Copy link
Member

Yeah, that's #135. Sorry for the noise.

@winterqt winterqt force-pushed the push-znwkmtytxwzt branch from 18804bc to 7381385 Compare June 22, 2025 20:45
@winterqt winterqt requested a review from zombiezen June 22, 2025 20:46
@winterqt
Copy link
Contributor Author

Since you squash merge PRs, I just put every refactor back into this commit, but if you'd rather me split it, let me know.

@winterqt winterqt force-pushed the push-znwkmtytxwzt branch from 7381385 to bbe2f30 Compare June 22, 2025 20:48
Copy link
Member

@zombiezen zombiezen left a comment

Choose a reason for hiding this comment

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

Thanks for the refactor! LGTM after the suggestions below (I think you should be able to 1-click all these in).

@zombiezen
Copy link
Member

Since you squash merge PRs, I just put every refactor back into this commit, but if you'd rather me split it, let me know.

That's fine. In general, I prefer just adding commits on top of the original so that the GitHub UI can show differences since last view. But as you note, it all gets squashed in, so it doesn't matter much.

@winterqt
Copy link
Contributor Author

Should be good now.

@zombiezen zombiezen merged commit 2b9bf99 into 256lights:main Jun 23, 2025
0 of 6 checks passed
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.

Add function for reading a file into a string

2 participants