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

[easy] Fix requirements json from #727 #729

Merged
merged 1 commit into from
Oct 21, 2022
Merged

Conversation

benjamin-shen
Copy link
Collaborator

@benjamin-shen benjamin-shen commented Oct 21, 2022

Summary

This pull request fixes the bad requirements json since #727 branched off an outdated source.

See the note in #628. Luckily it was easily caught since subsequent checks will fail (including when pushing to master). According to this comment, changing the job so it runs on PR instead of pulls might fix this issue.

Test Plan

CI/CD

@benjamin-shen benjamin-shen requested a review from a team as a code owner October 21, 2022 08:22
@dti-github-bot
Copy link
Member

[diff-counting] Significant lines: 0.

@github-actions
Copy link
Contributor

Visit the preview URL for this PR (updated for commit 2af7f6a):

https://cornelldti-courseplan-dev--pr729-ben-fix-reqs-json-oxo8yd73.web.app

(expires Sun, 20 Nov 2022 08:24:13 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 6d4e0437c4559ed895272bbd63991394f1e0e933

Copy link
Member

@noschiff noschiff left a comment

Choose a reason for hiding this comment

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

Thanks Ben! We should definitely look into running the checks on what the code will be after we've merged the PR. The only downside is that this could make it a little confusing to understand why a check failed since it'd be run on code not in the branch if the branch is behind master.

@benjamin-shen benjamin-shen merged commit be1389a into master Oct 21, 2022
@benjamin-shen benjamin-shen deleted the ben/fix-reqs-json branch October 21, 2022 17:59
noschiff pushed a commit that referenced this pull request Oct 26, 2022
This was referenced Oct 27, 2022
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