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 the bot and refactor files #6

Merged
merged 6 commits into from
Nov 17, 2019
Merged

Update the bot and refactor files #6

merged 6 commits into from
Nov 17, 2019

Conversation

mebeim
Copy link
Member

@mebeim mebeim commented Nov 14, 2019

So... not really a big change, just a bunch of little changes.

The main point of this PR is to update the bot to add a new endpoint that checks if a comment was already made on the PR before posting a new one. This will be helpful when adding the checking functionality in the main repo which checks if pages of a PR are already present or duplicated. To notify the author, the bot needs to comment only once when the PR is created, not every single time a commit is pushed to the PR (that functionality is still there of course since it's needed for build errors, just on a different endpoint).

Detailed list of changes (also available in the commit messages):

  • Changes to the Python script:

    1. Switch to Python 3.
    2. Rename post_comment.py to tldr_bot.py.
    3. Change main endpoint from / to /comment (old one is still kept for compatibility, will be removed once we update the travis scripts on the pages repo).
    4. Add new endpoint /comment/once which does the same as /comment, but first checks if tldr-bot hasn't already commented on the PR.
    5. Rename the /test endpoint to /status.
    6. Change HTTP response code for malformed request from 500 (server error) to 400 (client error).
    7. Remove the unneeded GITHUB_URL environment variable.
    8. Refactor code (reorder imports, indentation, spacing, etc.)
  • Changes to the systemd .service file:

    1. Change script name to tldr_bot.py.
    2. Add Wants= and After= directives to start the service after the network is online.
    3. Add recommended User= and Group= directives to run as a specific user different than root.
    4. Reorder fields in order of importance.
  • Changes to README.md:

    1. Change script name to tldr_bot.py.
    2. Remove unneeded GITHUB_URL and APP_ROOT environment variables.
    3. Overall refactor: wording, formatting, etc.

PS: I have no Idea why GitHub is using lowercase Roman numerals for the secodnary list elements, LOL. Hope it's still clear.

Changes in detail:

 1. Switch to Python 3.
 2. Rename `post_comment.py` to `tldr_bot.py`.
 3. Change main endpoint from `/` to `/comment` (old one is still
    kept for compatibility, will be removed once we update the
    travis scripts on the pages repo).
 4. Add new endpoint `/comment/once` which does the same as
    `/comment`, but first checks if `tldr-bot` hasn't already
    commented on the PR.
 5. Rename the `/test` endpoint to `/status`.
 6. Change HTTP response code for malformed request from 500 (server
    error) to 400 (client error).
 7. Remove the unneeded `GITHUB_URL` environment variable.
 8. Refactor code (reorder imports, indentation, spacing, etc.)
Changes in detail:

 1. Change script name to `tldr_bot.py`.
 2. Remove unneeded `GITHUB_URL` and `APP_ROOT` environment
    variables.
 3. Overall refactor: wording, formatting, etc.
Changes in detail:

 1. Change script name to `tldr_bot.py`.
 2. Add `Wants=` and `After=` directives to start the service after
    the network is online.
 3. Add recommended `User=` and `Group=` directives to run the
    as a specific user different than `root`.
 4. Reorder fields in order of importance.
Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

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

Great work @mebeim ! I have some comments to improve this further.

tldr_bot.py Show resolved Hide resolved
tldr_bot.py Outdated Show resolved Hide resolved
tldr_bot.py Outdated Show resolved Hide resolved
Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

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

Looks great @mebeim. Thanks !

@sbrl
Copy link
Member

sbrl commented Nov 16, 2019

@tldr-bot is now running the latest code from this branch.

tldr-bot.service Show resolved Hide resolved
@agnivade
Copy link
Member

Let's merge this !

@agnivade agnivade merged commit ed5ee77 into master Nov 17, 2019
@agnivade agnivade deleted the update branch November 17, 2019 03:22
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.

None yet

3 participants