Closed matklad closed 3 years ago
Thanks for PR. Looks great overall, I've left a couple of minor comments in review. It looks like our CI is having a bad day; I'll try to have a look at fixing it as soon as I have a free timeslot, then this PR may need to be rebased to see a green run.
Sure, no problem rebasing!
FYI, I see the "1 workflow awaiting approval" banner, if you want to disable for new contributors, you can check this box:
Figured out a much better solution -- there's no need for unsafety here.
Ok, https://github.com/tikv/rust-prometheus/pull/403 is merged so rebasing this PR now should make the CI happy.
@zhouqiang-cl @BusyJay @breeswish whoever of you has access to do that, can you please tweak the workflow permissions as shown in https://github.com/tikv/rust-prometheus/pull/402#issuecomment-877115118 so that we don't have to re-approve it on every push?
As github actions resource are bound to the organization account, so it might not be a good idea to just change it for a repository. Maybe you can send an issue to tikv/community or a thread at https://internals.tidb.io/, let's discuss it more!
@lucab ptal :)
I saw that we do
a bunch of times in near and figured that the library can probably guaranty utf8 validity and remove the need for one unwrap here.
This turned out to be slightly more involved then I've thought, as there's no standard way to explain that you'll only write utf8 to a
Write
, but it wasn't to hard either.As a bonus,
TextEncoder
impl now is statically guaranteed to produce only valid utf8