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

feat: add support for danish language #197

Merged
merged 14 commits into from
May 17, 2024
Merged

Conversation

Franta1205
Copy link
Contributor

I added Danish support, tested and is currently actively used by Danish-speaking people.

Copy link
Collaborator

@dblock dblock left a comment

Choose a reason for hiding this comment

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

It looks good! Undo the version changes please, and add .idea to .gitignore and remove the folder from the PR, it came along for the ride.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
numbers_and_words.gemspec Outdated Show resolved Hide resolved
numbers_and_words.gemspec Outdated Show resolved Hide resolved
spec/numbers_and_words/i18n_spec.rb Outdated Show resolved Hide resolved
@dblock
Copy link
Collaborator

dblock commented May 13, 2024

Looks like CI is failing, take a look?

Since this is a new feature we should increment the non-patch version from 0.11.13 to 0.12.0. Change the number wherever you find it?

@Franta1205
Copy link
Contributor Author

Looks like CI is failing, take a look?

Since this is a new feature we should increment the non-patch version from 0.11.13 to 0.12.0. Change the number wherever you find it?

Yes still failing will take look.

@dblock
Copy link
Collaborator

dblock commented May 14, 2024

Looks like CI is failing, take a look?
Since this is a new feature we should increment the non-patch version from 0.11.13 to 0.12.0. Change the number wherever you find it?

Yes still failing will take look.

Could it be this change in your PR?

https://github.com/kslazarev/numbers_and_words/pull/197/files#diff-c199102e7bb6ceb0d2a3bef3367a66b007a72213f06af0c8d4d20aa66c6ca426L221

@Franta1205
Copy link
Contributor Author

Looks like CI is failing, take a look?
Since this is a new feature we should increment the non-patch version from 0.11.13 to 0.12.0. Change the number wherever you find it?

Yes still failing will take look.

Could it be this change in your PR?

https://github.com/kslazarev/numbers_and_words/pull/197/files#diff-c199102e7bb6ceb0d2a3bef3367a66b007a72213f06af0c8d4d20aa66c6ca426L221

you're right could be. Reverted it back. Let's give it a try.

@dblock
Copy link
Collaborator

dblock commented May 14, 2024

I think this is as simple as bundle exec rake instead of rake in CI. I upgraded rubocop in #198, will merge that, rebase on top of it?

@Franta1205
Copy link
Contributor Author

I think this is as simple as bundle exec rake instead of rake in CI. I upgraded rubocop in #198, will merge that, rebase on top of it?

done

@dblock
Copy link
Collaborator

dblock commented May 15, 2024

Rubocop is complaining, run rubocop -a. Check that rubocop passes.

Double check that the specs include Danish. There are some things like FLOAT_CAPABLE_LANGUAGES that you may need to add it to as well.

@Franta1205
Copy link
Contributor Author

rubocop -a

I forgot to update the specs after solving some issues. Now all the tests pass correctly locally. Should be fine now. Sorry, I missed that.

@dblock
Copy link
Collaborator

dblock commented May 15, 2024

rubocop -a

I forgot to update the specs after solving some issues. Now all the tests pass correctly locally. Should be fine now. Sorry, I missed that.

No stress, but RuboCop still unhappy (https://github.com/kslazarev/numbers_and_words/actions/runs/9098488656/job/25014903295?pr=197). Try rubocop -a, really :)

@Franta1205
Copy link
Contributor Author

rubocop -a

I forgot to update the specs after solving some issues. Now all the tests pass correctly locally. Should be fine now. Sorry, I missed that.

No stress, but RuboCop still unhappy (https://github.com/kslazarev/numbers_and_words/actions/runs/9098488656/job/25014903295?pr=197). Try rubocop -a, really :)

yep, let's see if it's fine now

@dblock dblock merged commit 91f0f27 into kslazarev:master May 17, 2024
6 checks passed
@dblock
Copy link
Collaborator

dblock commented May 17, 2024

Merged, thank you!

I'd like to get #189 in and maybe #150 and make a release, maybe you can help with that one?

@Franta1205
Copy link
Contributor Author

Merged, thank you!

I'd like to get #189 in and maybe #150 and make a release, maybe you can help with that one?

Yes no prob will have look in to it

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.

None yet

2 participants