Skip to content

input-methods: add documentation#14602

Merged
joachifm merged 2 commits intoNixOS:masterfrom
ericsagnes:doc/input-methods
Apr 12, 2016
Merged

input-methods: add documentation#14602
joachifm merged 2 commits intoNixOS:masterfrom
ericsagnes:doc/input-methods

Conversation

@ericsagnes
Copy link
Contributor

Add basic documentation for input methods.
Address #13265.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @edolstra, @nbp and @ryanartecona to be potential reviewers

@joachifm
Copy link
Contributor

I like this. It's to the point and practical, makes sense even to a non-user. If I were to pick a nit (obligatory, right?), it'd be that nix-env -f '<nixpkgs>' is strictly unnecessary (or ought to be).

EDIT: also, if you decide to update the PR, could you use a more descriptive commit subject? Like manual: add chapter on input methods or something to that effect, that way it's immediately clear to someone reading the history that this change does not pertain to the package per se.

@joachifm joachifm added the 8.has: documentation This PR adds or changes documentation label Apr 11, 2016
@ericsagnes ericsagnes force-pushed the doc/input-methods branch 2 times, most recently from 5a41bbf to 536d16e Compare April 12, 2016 02:39
@ericsagnes
Copy link
Contributor Author

@joachifm Thanks for the review, I changed the nix-env part for a more descriptive text introducing each engine and updated the commit subject.
It might be worth to add something about manual related commits in the manual to have subject consistency for future commits.

Copy link
Member

Choose a reason for hiding this comment

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

No camelCase in filenames please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done in the input-method module: fix folder case commit.

@joachifm joachifm merged commit 852c85f into NixOS:master Apr 12, 2016
@ericsagnes
Copy link
Contributor Author

Thank you!


# ibus
(mkRenamedOptionModule [ "programs" "ibus" "plugins" ] [ "i18n" "inputMethod" "ibus" "engines" ])
(mkRenamedOptionModule [ "programs" "ibus" "plugins" ] [ "i18n" "input-method" "ibus" "engines" ])
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I didn't notice this one; this won't work, because you didn't actually change the option name (it is independent of the folder name).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joachifm Should I make a new PR to fix that or can it be fixed by amending the commit?

Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed a hotfix, no worries

Copy link
Contributor

Choose a reason for hiding this comment

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

This really is the kind of error that travis should pick up on ... oh well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

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

Labels

8.has: documentation This PR adds or changes documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants