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

Adding a GitHub action to perform a Binary Diff job on each PR/push event #14007

Merged
merged 24 commits into from
May 23, 2023

Conversation

zeusongit
Copy link
Contributor

@zeusongit zeusongit commented May 18, 2023

Purpose

Add a binary diff workflow that will run on each push and PR, this will enable devs to check for updated files (Added, Deleted or Modified) in their PR vs files on the master branch.

Note: caches will be removed automatically after 7 days.

Screenshot 2023-05-19 at 6 44 49 PM

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Add Bin Diff support on PRs

Reviewers

@DynamoDS/dynamo

@zeusongit zeusongit added DNM Do not merge. For PRs. WIP labels May 18, 2023
@zeusongit zeusongit changed the title [WIP] Adding a GitHub action to perform a Binary Diff job on each PR/push event Adding a GitHub action to perform a Binary Diff job on each PR/push event May 19, 2023
@zeusongit zeusongit removed DNM Do not merge. For PRs. WIP labels May 19, 2023
@@ -0,0 +1,258 @@
################################################################################
Copy link
Member

Choose a reason for hiding this comment

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

is this script based on some sample code? Did it have a license, can we link to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added source.

[string]$Path,

[parameter(HelpMessage="The hash algorithm to use.")]
[string]$Algorithm="MD5"
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if($Compare.length -ne 2) {
Print -x "Compare requires passing exactly 2 path parameters separated by comma, you passed $($Compare.length)." -f
}
Print "Comparing $($Compare[0]) to $($Compare[1])..." -a 1
Copy link
Member

Choose a reason for hiding this comment

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

so I have a bit of a hard time understanding how this code works - but from your screenshot I have a few questions/comments.

  1. I think noting that a file is modified if it still exists is probably just noise - IMO we don't really care, I expect files to be different when I do a build - they will contain different version numbers etc.
  2. What I really care about are new files and deleted files - if we need to keep modified files - can we make deleted and added files pop out some how? (color text?)
  3. Is a deleted file also considered modified or does it have a separate state? Can you show a screenshot of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Agreed, do you suggest removing modified entries? I think we can let them be.
  2. I have added colors to make them pop, green for Added, and red for Deleted entries.
  3. Yes, deleted files has a separate "Deleted" label. attaching ss below.
    Screenshot 2023-05-22 at 7 22 26 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed hash and side indicator columns as they were not useful.

Screenshot 2023-05-22 at 8 32 56 PM

build-current:
runs-on: windows-2022
steps:
- name: Checkout Dynamo Repo current branch
Copy link
Member

Choose a reason for hiding this comment

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

I have a complication for you - we also need to do this same comparison for the DynamoAll.Net6.Sln targeting windows.

Would you prefer to just duplicate this action for net6 or have this job do 4 builds?

Copy link
Contributor Author

@zeusongit zeusongit May 23, 2023

Choose a reason for hiding this comment

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

Added 2 more builds for Net6-Windows (parallel builds for quick results)

Screenshot 2023-05-22 at 8 34 31 PM

@QilongTang QilongTang merged commit 44e4459 into DynamoDS:master May 23, 2023
@zeusongit
Copy link
Contributor Author

@mjkkirschner I don't think the coloring worked in Github Action console logs, but it did work in powershell.

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