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.

Sign up for GitHub

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

Jump to bottom

templater: add Email template type, deprecate Signature.username() #5079

Merged
merged 1 commit into from
Dec 15, 2024
Merged

templater: add Email template type, deprecate Signature.username() #5079

merged 1 commit into from
Dec 15, 2024
+148 −19

Conversation

bnjmnt4n
Copy link
Collaborator

The Signature.email() method is also updated to return the new Email type. The Signature.username() method is deprecated for Signature.email().local().

This is an alternative to #4687.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

@bnjmnt4n bnjmnt4n force-pushed the bnjmnt4n/push-knznzkuuumll branch from aa7ae95 to eee3065 Compare December 12, 2024 03:47
yuja
yuja approved these changes Dec 12, 2024
View reviewed changes
Copy link
Collaborator

@yuja yuja left a comment

Choose a reason for hiding this comment

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

Thanks.

cli/src/template_builder.rs Outdated Show resolved Hide resolved
cli/src/template_builder.rs Outdated Show resolved Hide resolved
cli/src/templater.rs Outdated Show resolved Hide resolved
docs/templates.md Outdated Show resolved Hide resolved
@bnjmnt4n bnjmnt4n force-pushed the bnjmnt4n/push-knznzkuuumll branch 3 times, most recently from f30ac35 to 94d0071 Compare December 13, 2024 03:53
@bnjmnt4n
Copy link
Collaborator Author

A potential issue of this change is that you can no longer simply compare author.email() == "[email protected]", unless we implement that as well. Any thoughts on whether we should do that?

@yuja
Copy link
Collaborator

yuja commented Dec 13, 2024

A potential issue of this change is that you can no longer simply compare author.email() == "[email protected]", unless we implement that as well. Any thoughts on whether we should do that?

I think it's okay to implement == for types which have one obvious type conversion. We could add email(String) -> Email, stringify(Template) -> String, or email.to_string(), but that would be tedious to use.

@bnjmnt4n bnjmnt4n force-pushed the bnjmnt4n/push-knznzkuuumll branch from 94d0071 to e2c6fa3 Compare December 13, 2024 10:45
@bnjmnt4n
Copy link
Collaborator Author

I think it's okay to implement == for types which have one obvious type conversion.

I've updated the PR to add == for Email and String comparison as well.

@bnjmnt4n bnjmnt4n force-pushed the bnjmnt4n/push-knznzkuuumll branch from e2c6fa3 to a76fbd0 Compare December 15, 2024 06:54
yuja
yuja approved these changes Dec 15, 2024
View reviewed changes
cli/src/template_builder.rs Show resolved Hide resolved
@bnjmnt4n
templater: add Email template type, deprecate Signature.username()
51b7895 
The `Signature.email()` method is also updated to return the new Email
type. The `Signature.username()` method is deprecated for
`Signature.email().local()`.
@bnjmnt4n bnjmnt4n force-pushed the bnjmnt4n/push-knznzkuuumll branch from a76fbd0 to 51b7895 Compare December 15, 2024 07:51
@bnjmnt4n bnjmnt4n enabled auto-merge (rebase) December 15, 2024 08:02
@bnjmnt4n bnjmnt4n merged commit dc16830 into main Dec 15, 2024
31 checks passed
@bnjmnt4n bnjmnt4n deleted the bnjmnt4n/push-knznzkuuumll branch December 15, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuja yuja yuja approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bnjmnt4n @yuja