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

Report PR changes phase B #1294

Merged
merged 8 commits into from
Nov 27, 2023
Merged

Conversation

dachengx
Copy link
Collaborator

Organize codes into python script but there is no functionality changed.

bash .github/scripts/check_lineage_updates.sh track_lineage_changes test_plugin_changes

@dachengx dachengx marked this pull request as ready for review November 23, 2023 02:26
@dachengx dachengx requested a review from JYangQi00 November 23, 2023 02:27
@dachengx dachengx changed the title Report pr changes phase b Report PR changes phase B Nov 23, 2023
if [ "$#" -ne 4 ]; then
echo "Usage: $0 <first_branch_name> <second_branch_name> <output folder path> <run_id>"
if [ "$#" -ne 2 ]; then
echo "Usage: $0 <first_branch_name> <second_branch_name>"
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably all of the files generated from this script are just saved in the current directory, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, currently yes.

./report_pr_changes --branch new --computation changed_affected_plugin
git checkout $old_branch
./report_pr_changes --branch old --computation changed_affected_plugin
./report_pr_changes --branch old --computation report_changes
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the cleanup function automatically delete all of the temporary json files that we've made?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I have not deleted them. Maybe should delete it by python scripts. Because they are generated by the python scripts.

@@ -0,0 +1,203 @@
#!/usr/bin/env python
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for organizing the code! I really appreciate it. Shall I comment these functions?

Copy link
Contributor

@JYangQi00 JYangQi00 left a comment

Choose a reason for hiding this comment

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

Thanks Dacheng, everything makes sense and looks good to me. For record-keeping, we will meet later today to discuss this PR and if we agree that this looks good and works well then I will approve.

@JYangQi00 JYangQi00 merged commit 3d0e86a into report_pr_changes Nov 27, 2023
8 checks passed
@dachengx dachengx deleted the report_pr_changes_phase_b branch January 17, 2024 14:26
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.

2 participants