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

Fix #236 and #313: Remove unnecessary comment about modifications to codeowner file; notify server admins about changes to files in jobs and platform folders. #332

Merged
merged 19 commits into from
Aug 2, 2023

Conversation

vojtechjelinek
Copy link
Member

@vojtechjelinek vojtechjelinek commented Jul 28, 2023

Explanation

Fix #236: Remove the comment about modifications to codeowner file as it is unnecessary.
Fix #313: Notify server admins about any changes to non-test files in jobs and platform folders.

Checklist

  • I have successfully deployed my own instance of Oppiabot.
    • You can find instructions for doing this here.
  • I have manually tested all the changes made in this PR following the manual tests matrix.

@vojtechjelinek vojtechjelinek marked this pull request as ready for review July 30, 2023 17:20
Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

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

Thanks @vojtechjelinek! Just a couple small comments, otherwise LGTM.

plural: 'jobs'
},
JOB_REGEX
const fileChangesShouldBeVerified = (filename) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggest shouldFileChangesBeVerified (start with a "verb")

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@@ -517,4 +516,4 @@ describe('Cron Job Spec', () => {
expect(github.issues.createComment).not.toHaveBeenCalled();
});
});
});
});
Copy link
Member

Choose a reason for hiding this comment

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

Missing newline at EOF

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@seanlip seanlip assigned vojtechjelinek and unassigned seanlip Jul 31, 2023
@seanlip seanlip changed the title Add review files Fix #236 and #313: Remove unnecessary comment about modifications to codeowner file; notify server admins about changes to files in jobs and platform folders. Jul 31, 2023
@vojtechjelinek vojtechjelinek merged commit 008ba60 into oppia:master Aug 2, 2023
3 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
3 participants