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

Implement --show-action command line argument #392

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Jasha10
Copy link
Contributor

@Jasha10 Jasha10 commented Apr 10, 2021

Related to #390

@Jasha10
Copy link
Contributor Author

Jasha10 commented Apr 10, 2021

@schettino72 let me know if you have any feedback :)

@schettino72
Copy link
Member

  • add new command line option --show-action (better name suggestions welcome)
  • implement a title() method for PythonAction & CmdAction
  • Reporter will also need a new method to be called before each action

keep going 😄

@Jasha10
Copy link
Contributor Author

Jasha10 commented Apr 13, 2021

* [ ]  Reporter will also need a new method to be called before each action

I am confused about this. The reporter is not looping over actions. The for action in ... loop is inside task.py:Task.execute:

    def execute(self, stream):
        ...
        task_stdout, task_stderr = stream._get_out_err(self.verbosity)
        for action in self.actions:
            action_return = action.execute(task_stdout, task_stderr)
            ...

@schettino72
Copy link
Member

So we need to make Task.execute call the reporter for each action. Every output must be through a reporter.

@codecov-commenter
Copy link

codecov-commenter commented Apr 18, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@9efe141). Click here to learn what that means.
The diff coverage is 96.07%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #392   +/-   ##
=========================================
  Coverage          ?   99.70%           
=========================================
  Files             ?       65           
  Lines             ?     8898           
  Branches          ?        0           
=========================================
  Hits              ?     8872           
  Misses            ?       26           
  Partials          ?        0           
Impacted Files Coverage Δ
doit/action.py 99.24% <80.00%> (ø)
doit/reporter.py 100.00% <100.00%> (ø)
doit/runner.py 100.00% <100.00%> (ø)
doit/task.py 100.00% <100.00%> (ø)
tests/test_action.py 100.00% <100.00%> (ø)
tests/test_reporter.py 100.00% <100.00%> (ø)
tests/test_task.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9efe141...c192882. Read the comment docs.

@Jasha10
Copy link
Contributor Author

Jasha10 commented Apr 18, 2021

So we need to make Task.execute call the reporter for each action. Every output must be through a reporter.

I am planning to add a reporter argument to the Task.execute function signature, so that Task.execute can access the reporter.

@Jasha10 Jasha10 marked this pull request as draft April 18, 2021 22:36
@Jasha10 Jasha10 changed the title Implement CmdAction.title() and PythonAction.title() Implement --show-action command line argument Apr 18, 2021
@Jasha10
Copy link
Contributor Author

Jasha10 commented Apr 19, 2021

I was only able to support ConsoleReporter. I tried to support JsonReporter but there were lots of exceptions related to task_dispatcher that I was't able to debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants