Sloppy language #224

Merged
dianawi merged 2 commits from sloppy-language into master 2019-04-09 08:44:28 +00:00
dianawi commented 2019-03-02 15:53:48 +00:00 (Migrated from github.com)
No description provided.
matthijskooijman (Migrated from github.com) requested changes 2019-03-11 20:06:27 +00:00
matthijskooijman (Migrated from github.com) left a comment

Most of these changes look good. I added some inline comments. Also, login-username, login-password and add-mailing are introduced as strings in the first commit, but only used in the second commit. It would be better to group both adding and using these strings in a single commit (ideally in a separate commit from the other string changes, but putting everything in a single commit would also be ok for me, since it all improves or adds translations).

Most of these changes look good. I added some inline comments. Also, `login-username`, `login-password` and `add-mailing` are introduced as strings in the first commit, but only used in the second commit. It would be better to group both adding and using these strings in a single commit (ideally in a separate commit from the other string changes, but putting everything in a single commit would also be ok for me, since it all improves or adds translations).
matthijskooijman (Migrated from github.com) commented 2019-03-11 20:02:11 +00:00

I can't find any reference to this string in the code?

I can't find any reference to this string in the code?
@ -15,3 +15,3 @@
"logout" => "uitloggen",
"quit" => "stop",
"quit" => "stoppen",
"logged-in-as" => "ingelogd als",
matthijskooijman (Migrated from github.com) commented 2019-03-11 19:44:28 +00:00

I think this conflicts with (and is more elaborately fixed by) #218, so probably best to remove this change from this commit.

I think this conflicts with (and is more elaborately fixed by) #218, so probably best to remove this change from this commit.
matthijskooijman (Migrated from github.com) reviewed 2019-03-11 20:09:43 +00:00
matthijskooijman (Migrated from github.com) commented 2019-03-11 20:09:43 +00:00

Ah, seems this was added in #227 (both the string definition and the usage). I think this should be dropped from this commit.

Ah, seems this was added in #227 (both the string definition and the usage). I think this should be dropped from this commit.
matthijskooijman (Migrated from github.com) reviewed 2019-03-11 20:10:48 +00:00
matthijskooijman (Migrated from github.com) commented 2019-03-11 20:10:48 +00:00

Scratch that, it seems #227 only adds the strings, but does not use them anywhere...

Scratch that, it seems #227 only adds the strings, but does not use them anywhere...
tammoterhark (Migrated from github.com) reviewed 2019-03-30 10:07:15 +00:00
tammoterhark (Migrated from github.com) commented 2019-03-30 10:07:14 +00:00

Looks to me like ml refers to mailinglists. Can they have comments?

Looks to me like `ml` refers to mailinglists. Can they have comments?
laurensmartina (Migrated from github.com) approved these changes 2019-03-30 12:54:02 +00:00
matthijskooijman commented 2019-04-08 20:57:36 +00:00 (Migrated from github.com)

Looks good at first glance. I think this can be merged after testing.

Looks good at first glance. I think this can be merged after testing.
matthijskooijman (Migrated from github.com) approved these changes 2019-04-08 21:02:31 +00:00
matthijskooijman commented 2019-04-09 08:44:23 +00:00 (Migrated from github.com)

Tested, works. I rebased on top of master, merging now.

Tested, works. I rebased on top of master, merging now.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
harmen/hypha!224
No description provided.