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.

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

Add Vietnamese locale #550

Merged
merged 5 commits into from
Sep 27, 2023
Merged

Add Vietnamese locale #550

merged 5 commits into from
Sep 27, 2023

Conversation

YutoYasunaga
Copy link
Contributor

Thank you for the great project,
I've added Vietnamese locale

Result:

スクリーンショット 2023-09-01 21 04 56

スクリーンショット 2023-09-01 21 04 25

スクリーンショット 2023-09-01 21 06 43 スクリーンショット 2023-09-01 21 04 34

@benkoshy
Copy link
Collaborator

benkoshy commented Sep 2, 2023

YutoYasunaga thank you for your contribution!

It will take some time for me to assess / understand how locales work etc.

I will get back to you in time.

@benkoshy
Copy link
Collaborator

benkoshy commented Sep 5, 2023

Hi Yuto

Thank you for your contribution! Apologies for the delay.

A couple of things are are required to finalize the merge. (Noted here: https://github.com/ddnexus/pagy/blob/master/lib/locales/README.md)

@YutoYasunaga
Copy link
Contributor Author

Thank you for your feedback and detailed guidance. apologies for the delay.

I have made the requested changes and updated the pull request. Please review and let me know if there are any further modifications needed.

@benkoshy
Copy link
Collaborator

thx @YutoYasunaga

I will review this next week: by the 2023-09-21.

@benkoshy
Copy link
Collaborator

benkoshy commented Sep 21, 2023

Thank you for your patience. It looks good to me (even though I don't understand the language 😂 ). I have had a closer look at tested it out on my own repo:

multiple_pages_2
multiple_pages
single_page
zero

The only issue now is working out why the CI is failing.

I will try to have a look by tomorrow.

@ddnexus
Copy link
Owner

ddnexus commented Sep 21, 2023

@benkoshy It's missing the new file in the manifest. You can run the manifest:update task and commit.

Which is another point missed in the doc 🤷

@benkoshy
Copy link
Collaborator

@YutoYasunaga if you run the following command:

rake manifest:generate

and then confirm it with:

rake manifest:check

You can then add another commit and the CI will pass. Or you can cherry pick this commit: YutoYasunaga@68ae38b

And I think that will be all that is required.

@YutoYasunaga
Copy link
Contributor Author

Thank for your help.

I pushed it already.

@benkoshy
Copy link
Collaborator

Rubocop is complaining. You can cherry-pick: ff4cd69

@YutoYasunaga
Copy link
Contributor Author

@benkoshy Thank you. I have done it.

image

@ddnexus ddnexus merged commit 3d5dfbb into ddnexus:dev Sep 27, 2023
3 checks passed
@ddnexus
Copy link
Owner

ddnexus commented Sep 27, 2023

Thank you @YutoYasunaga @benkoshy

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.

3 participants