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

Update output display to job summary #3914

Merged
merged 3 commits into from Oct 6, 2023
Merged

Conversation

csalerno-asml
Copy link
Contributor

@csalerno-asml csalerno-asml commented Oct 2, 2023

Description

Black default job summary isn't really readable, since Python code will be converted 1:1 in Markdown, leading to something like this: https://github.com/csalerno-asml/test-black-repo/actions/runs/6380983558#summary-17316480770

With this proposal, the diff will be wrapped in a triple-quoted python box: https://github.com/csalerno-asml/test-black-repo/actions/runs/6380983558#summary-17316481072

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

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

I like this idea. Makes a lot of sense.

I do wonder tho, can we break people who might be already parsing the output? If so, should we consider making a config parameter (if possible) to disable this code block new default? (I don't know how easy this is to do)

@csalerno-asml
Copy link
Contributor Author

Hi @cooperlees , thanks for the feedback!

Regarding your concern, I am not really sure that somebody can/will parse the output?
I think the only use case with summaries is: an action/step write to GITHUB_STEP_SUMMARY, and a human read it via UI.

If a user wants to read the output of black, usually he/she will use black via CLI or with a pre-commit hook

@cooperlees cooperlees added skip news Pull requests that don't need a changelog entry. and removed skip news Pull requests that don't need a changelog entry. labels Oct 3, 2023
Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

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

Ok, I thought this was for human consumption only, was just checking we were not missing anything.

In that case lets add a change log entry so we show somewhere we've decided to make this change and rebase this and I'll merge.

Thankyou for your contribution!

@csalerno-asml
Copy link
Contributor Author

csalerno-asml commented Oct 5, 2023

Hey @cooperlees , I noticed that the exit code was not handled correctly, so I updated the PR

@cooperlees
Copy link
Collaborator

Many thanks again. This will help people see the changes to the code etc.

@cooperlees cooperlees merged commit 3457ec4 into psf:main Oct 6, 2023
38 checks passed
@csalerno-asml csalerno-asml deleted the patch-1 branch October 11, 2023 08:12
@heiner
Copy link

heiner commented Oct 19, 2023

I believe this change might have caused the GitHub action to fail without actionable output in some cases, e.g. in a way where the only output is:

Run psf/black@stable
  with:
    version: 23.10.0
    options: --check --diff
    src: .
    jupyter: false
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.9.18/x64
Run if [ "$RUNNER_OS" == "Windows" ]; then
  if [ "$RUNNER_OS" == "Windows" ]; then
    runner="python"
  else
    runner="python3"
  fi
  
  out=$(${runner} $GITHUB_ACTION_PATH/action/main.py)
  exit_code=$?
  
  # Display the raw output in the step
  echo "${out}"
  
  # Display the Markdown output in the job summary
  echo "\`\`\`python" >> $GITHUB_STEP_SUMMARY
  echo "${out}" >> $GITHUB_STEP_SUMMARY
  echo "\`\`\`" >> $GITHUB_STEP_SUMMARY
  
  # Exit with the exit-code returned by Black
  exit ${exit_code}
  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.9.18/x64
    INPUT_OPTIONS: --check --diff
    INPUT_SRC: .
    INPUT_JUPYTER: false
    INPUT_BLACK_ARGS: 
    INPUT_VERSION: 23.10.0
    pythonioencoding: utf-8
Error: Process completed with exit code 1.

@csalerno-asml csalerno-asml mentioned this pull request Oct 20, 2023
3 tasks
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