AstraLuma / xontrib-z

Tracks your most used directories, based on 'frecency'.
GNU General Public License v3.0
24 stars 16 forks source link

Fix «Event handlers need a **kwargs for future proofing» #4

Closed StreakyCobra closed 7 years ago

StreakyCobra commented 7 years ago

As indicated by this Trace when xonsh is run with $XONSH_DEBUG=1

Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/xonsh/proc.py", line 1304, in run
    r = self.f(self.args, sp_stdin, sp_stdout, sp_stderr, spec)
  File "/usr/lib/python3.6/site-packages/xonsh/proc.py", line 1144, in proxy_two
    return f(args, stdin)
  File "/usr/lib/python3.6/site-packages/xonsh/xontribs.py", line 169, in xontribs_main
    return _MAIN_XONTRIB_ACTIONS[ns.action](ns)
  File "/usr/lib/python3.6/site-packages/xonsh/xontribs.py", line 87, in _load
    update_context(name, ctx=ctx)
  File "/usr/lib/python3.6/site-packages/xonsh/xontribs.py", line 66, in update_context
    modctx = xontrib_context(name)
  File "/usr/lib/python3.6/site-packages/xonsh/xontribs.py", line 33, in xontrib_context
    m = importlib.import_module(spec.name)
  File "/usr/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 978, in _gcd_import
  File "<frozen importlib._bootstrap>", line 961, in _find_and_load
  File "<frozen importlib._bootstrap>", line 950, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 655, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 205, in _call_with_frames_removed
  File "/home/user/.local/lib/python3.6/site-packages/xontrib/z.py", line 208, in <module>
    @events.on_chdir
  File "/usr/lib/python3.6/site-packages/xonsh/events.py", line 70, in __call__
    raise ValueError("Event handlers need a **kwargs for future proofing")
ValueError: Event handlers need a **kwargs for future proofing

Signed-off-by: Fabien Dubosson fabien.dubosson@gmail.com

AstraLuma commented 7 years ago

I'm actually going to not merge this because there's other things that need to happen. But thank you very much for reminding me I need to do them and submitting a PR.

StreakyCobra commented 7 years ago

Ok :+1:

Side note: Do action like this very carefully, i.e. closing valid Pull Request saying «I'm going to do it myself for/because …», especially with newcomers.

It is what makes the difference between newcomers to stay with "Nice, let's try another contribution", and making newcomers to leave with "Ok, so do it by yourself, bye". Even if it is obvious and really simple changes like this one. Contributors, especially newcomers, took a lot of time to: enter and understand the code, fork the project, clone it locally, create a branch, make a change, write a commit message, push back and open a pull-request. By accepting a PR, you recognize the time, the work and the interest they have put on your project. This will also show on their GitHub profile they have contributed to your project, what is a nice reward for them.

In a case like this one, you can simply cherry-pick the commit and do your other changes in an additional commit just behind. It happens that sometime a PR is not perfect. If it's code-wise, simply ask for changes during the PR review, or accept the PR and fix it in an additional commit. If it is commit-message-wise, I happened to have reworded some when cherry-picking, but always by keeping the original author.

In this case don't worry :smile: I'm used to open source project contributions and I've already contributed to enough of them for not having to care about one particular contribution.