Skip to content
This repository was archived by the owner on Jan 29, 2024. It is now read-only.

Add support for yml filenames where locale is used as prefix#22

Open
1st8 wants to merge 1 commit intorelatel:masterfrom
mixxt:support-different-naming-schema
Open

Add support for yml filenames where locale is used as prefix#22
1st8 wants to merge 1 commit intorelatel:masterfrom
mixxt:support-different-naming-schema

Conversation

@1st8
Copy link
Copy Markdown
Contributor

@1st8 1st8 commented Oct 21, 2015

No description provided.

@1st8
Copy link
Copy Markdown
Contributor Author

1st8 commented Nov 23, 2015

Any thoughts about this? I don't want to annoy anyone, but please don't leave a small and focused PR hanging like this 😉

@1st8 1st8 force-pushed the support-different-naming-schema branch from dee1b40 to dd4cf0c Compare November 23, 2015 09:11
Comment thread lib/i18n_yaml_editor/store.rb Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Find these regexes really tough to read, when what we're trying to do is remove the -2 constant. How about:

parts = File.basename(translation.file).split('.')
parts[parts.index(translation.locale)] = locale

path = "#{File.dirname(translation.file)/#{parts.join('.')}"

😁

@kaspth
Copy link
Copy Markdown
Contributor

kaspth commented Nov 23, 2015

@1st8 Ha, had that coming 😄

@1st8 1st8 force-pushed the support-different-naming-schema branch 2 times, most recently from af4ab30 to e243cd3 Compare November 26, 2015 09:45
Extract path transformation helper #replace_locale_in_path
@1st8 1st8 force-pushed the support-different-naming-schema branch from e243cd3 to af0e334 Compare November 26, 2015 10:14
@1st8
Copy link
Copy Markdown
Contributor Author

1st8 commented Nov 26, 2015

@kaspth Hehe, yeah 😉
Your suggested code is better, indeed. Changed the implementation to use it and extracted it out into a method to make it more readable. Updated the specs as well.

Comment thread test/unit/test_store.rb
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't see the point in testing argument positions, sounds like a bag of hurt going forward 😁

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am testing, that the interface defined by Transformation. replace_locale_in_path is used and used correctly. Otherwise I can't be sure, that it is actually used and not being replaced with something inline, which would make my encapsulated tests obsolete.

@kaspth
Copy link
Copy Markdown
Contributor

kaspth commented Nov 26, 2015

Coming along nicely 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants