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

Support char type #128

Merged
merged 1 commit into from
Jul 13, 2023
Merged

Support char type #128

merged 1 commit into from
Jul 13, 2023

Conversation

czocher
Copy link
Contributor

@czocher czocher commented Jun 19, 2023

Add support for the Rust char type.

Closes #103

@Lucretiel
Copy link
Contributor

Looks great, just a small Swift nit and it's good to merge

@czocher
Copy link
Contributor Author

czocher commented Jun 24, 2023

@Lucretiel already fixed ;).

@Lucretiel
Copy link
Contributor

I'm not seeing it on the PR; does it need to be pushed?

@czocher
Copy link
Contributor Author

czocher commented Jun 29, 2023

I'm not seeing it on the PR; does it need to be pushed?

@Lucretiel You mean the change requested by @CerulanLumina? I amended the change just after they noticed it. This is the current version that shows up when I check the changes in this PR: https://github.com/1Password/typeshare/pull/128/files#diff-e3f812823b9b43de87bbd7c54ab61d3e102c387d74245c65cd8a0e3bbca2193e, is there some other thing I missed?

@Lucretiel
Copy link
Contributor

Whoops, it was I who didn't didn't press Push on my review and just let it sit as a draft

Lucretiel
Lucretiel requested changes Jul 1, 2023
View reviewed changes
core/src/language/swift.rs Outdated Show resolved Hide resolved
@czocher
Copy link
Contributor Author

czocher commented Jul 1, 2023

@Lucretiel done!

@LuminaSapphira
Copy link
Collaborator

Looks good! Just need the commit to pass signing then we'll merge!

@czocher
Copy link
Contributor Author

czocher commented Jul 8, 2023

There was some weird mixup with my GitHub account configuration, reuploaded my GPG key and it seems to work. Thanks!

@LuminaSapphira LuminaSapphira merged commit 5ae4e1e into 1Password:main Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LuminaSapphira LuminaSapphira LuminaSapphira left review comments

@Lucretiel Lucretiel Lucretiel 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.

Support for char
3 participants
@czocher @Lucretiel @LuminaSapphira