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

spellcheck: add examples #1545

Merged
merged 1 commit into from
Jun 11, 2019
Merged

spellcheck: add examples #1545

merged 1 commit into from
Jun 11, 2019

Conversation

torstehu
Copy link
Contributor

@torstehu torstehu commented Apr 5, 2019

No description provided.

@HumorBaby
Copy link
Contributor

HumorBaby commented Apr 5, 2019

Since this is a spellcheck module, the typo is actually intentional 😄 If the command is run as is shown in the example (with "typo"), it will demonstrate how the bot responds to incorrectly spelled wrods words

@HumorBaby
Copy link
Contributor

It may be helpful to add examples outputs to the @example decoration to 1) prevent such confusion regarding the examples, and 2) show what expected output is when (a) wrod is spelled incorrectly, and (b) word is spelled correctly.

The relevant arguments to the @example decorator:

sopel/sopel/module.py

Lines 428 to 440 in 95812b5

class example(object):
"""Decorate a function with an example.
Args:
msg:
(required) The example command as sent by a user on IRC. If it is
a prefixed command, the command prefix used in the example must
match the default `config.core.help_prefix` for compatibility with
the built-in help module.
result:
What the example command is expected to output. If given, a test is
generated using `msg` as input. The test behavior can be modified
by the remaining optional arguments.

In use:

@example('.shrug', r'¯\_(ツ)_/¯')

So, @torstehu, building off your current suggested change, adding another @example with wrod and adding expected results for both @examples are needed for this PR, IMO.

@torstehu torstehu force-pushed the fix-typo branch 2 times, most recently from d071412 to 7d9d468 Compare April 5, 2019 14:55
@torstehu
Copy link
Contributor Author

torstehu commented Apr 5, 2019

@HumorBaby Thanks for the clarification, I have tried to make some examples with one and two words, can you give me some comments on this commit?

@torstehu torstehu changed the title spellcheck: fix typo spellcheck: add examples Apr 5, 2019
@dgw
Copy link
Member

dgw commented Apr 5, 2019

I'm confused now, because I thought I included example tests in #1164. Wonder what happened to them… *digs around local repos*

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

The examples I wrote must be stashed on my office computer (which I probably won't see again until Monday)—or I accidentally reset the Git worktree and lost them.

Either way, these are very similar to what I had.

@dgw dgw added this to the 7.0.0 milestone Apr 5, 2019
@dgw dgw added the Tests label Apr 5, 2019
@Exirel
Copy link
Contributor

Exirel commented Apr 9, 2019

All these look good! 🎉 :shipit:

@dgw
Copy link
Member

dgw commented Apr 9, 2019

The examples I wrote must be stashed on my office computer

Or not. I have no idea where they went, then.

@Exirel did you want to formally approve this, or let me take full responsibility? :P

@Exirel
Copy link
Contributor

Exirel commented Apr 10, 2019

let me take full responsibility? :P

Yes. 😛

@Exirel
Copy link
Contributor

Exirel commented May 8, 2019

@dgw I wonder if I should create a label "easy to merge" for you to check... ;-)

@dgw
Copy link
Member

dgw commented May 8, 2019

@Exirel It wouldn't speed up my merging pace, lol. I just don't like merging too much at once.

@dgw dgw merged commit 3015b08 into sopel-irc:master Jun 11, 2019
@torstehu torstehu deleted the fix-typo branch June 12, 2019 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants