Skip to content

Add Base64Url encoding - #56

Merged
JordanMartinez merged 3 commits into
masterfrom
update-encoding
Jul 5, 2023
Merged

Add Base64Url encoding#56
JordanMartinez merged 3 commits into
masterfrom
update-encoding

Conversation

@JordanMartinez

@JordanMartinezJordanMartinez commented Jul 5, 2023

Copy link
Copy Markdown
Contributor

Description of the change

  • Add Base64Url encoding.

Checklist:

  • Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

@JordanMartinezJordanMartinez changed the title update encodingAdd Base64Url encoding; drop aliased encodingsJul 5, 2023
@JordanMartinez

Copy link
Copy Markdown
ContributorAuthor

@thomashoneyman 🏓

@thomashoneyman

Copy link
Copy Markdown
Contributor

Has Node indicated that they will drop these aliases? What harm do you see in retaining them?

@JordanMartinez

Copy link
Copy Markdown
ContributorAuthor

Has Node indicated that they will drop these aliases? What harm do you see in retaining them?

The UCS2 one not so much, but the binary one is confusing. Per Node docs

'binary': Alias for 'latin1'. See binary strings for more background on this topic. The name of this encoding can be very misleading, as all of the encodings listed here convert between strings and binary data (emphasis mine). For converting between strings and Buffers, typically 'utf8' is the right choice.

@JordanMartinez

Copy link
Copy Markdown
ContributorAuthor

Regardless, this is one of those "controversial" changes, and I don't want to be blocked by this. I'll revert the commit.

@JordanMartinezJordanMartinez changed the title Add Base64Url encoding; drop aliased encodingsAdd Base64Url encodingJul 5, 2023
@JordanMartinez
JordanMartinez merged commit c447cb5 into masterJul 5, 2023
@JordanMartinez
JordanMartinez deleted the update-encoding branch July 5, 2023 20:44
Sign up for freeto 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

@JordanMartinez@thomashoneyman