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

[Github] Fix github automation script on empty descriptions #71274

Merged

Conversation

boomanaiden154
Copy link
Contributor

Before this patch, the github automation script would fail when trying to escape the text of a PR/issue description that was empty as the Github library returns None instead of an empty string in those scenarios. This patch special cases this and makes the escape function return an empty string when given a value of None.

Before this patch, the github automation script would fail when trying
to escape the text of a PR/issue description that was empty as the
Github library returns None instead of an empty string in those
scenarios. This patch special cases this and makes the escape function
return an empty string when given a value of None.
@boomanaiden154
Copy link
Contributor Author

This addresses failures like the one present in https://github.com/llvm/llvm-project/actions/runs/6753398365/job/18360167668:

Traceback (most recent call last):
  File "/home/runner/work/llvm-project/llvm-project/./github-automation.py", line 702, in <module>
    pr_subscriber.run()
  File "/home/runner/work/llvm-project/llvm-project/./github-automation.py", line 163, in run
    body = escape_description(self.pr.body)
  File "/home/runner/work/llvm-project/llvm-project/./github-automation.py", line 52, in escape_description
    str = html.escape(str, False)
  File "/usr/lib/python3.10/html/__init__.py", line 1[9](https://github.com/llvm/llvm-project/actions/runs/6753398365/job/18360167668#step:3:10), in escape
    s = s.replace("&", "&amp;") # Must be done first!
AttributeError: 'NoneType' object has no attribute 'replace'

Not sure if this is the most optimal way to handle it (might want to handle it outside of the escape function), but I'm happy to implement feedback from reviewers here.

Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.

@M4ximumPizza
Copy link

Fantastic fix. Thanks!

@boomanaiden154
Copy link
Contributor Author

@M4ximumPizza Can you please stop reviewing random PRs unless you have something to add to the conversation? Thanks!

@boomanaiden154 boomanaiden154 merged commit 5d39f3f into llvm:main Nov 7, 2023
2 checks passed
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.

3 participants