Skip to content

chore(ci): correct the direction of diff equations #2396

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 4, 2024
Merged

chore(ci): correct the direction of diff equations #2396

merged 1 commit into from
Jan 4, 2024

Conversation

castastrophe
Copy link
Collaborator

@castastrophe castastrophe commented Jan 2, 2024 •
edited
Loading

Description

This is clarifying some of the equations used to determine file diffs and creates a clearer variable naming for the head and base branches being evaluated.

  • path renamed to head-path
  • diff-path renamed to base-path
  • this same renaming is carried throughout the file-diff/index.js file
  • before the summary table, I added the following note: Table below reports only on changes to a package's main file. Other changes can be found in the collapsed "Details" below.
  • fixed the summary table to report on changes to the main file (before it was reported whatever the last file in the list was)
  • more comments throughout

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

To-do list

  • I have read the contribution guidelines.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 2, 2024 •
edited
Loading

🚀 Deployed on https://pr-2396--spectrum-css.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 2, 2024 •
edited
Loading

File metrics

Summary

Total size: 3.97 MB*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@castastrophe castastrophe force-pushed the chore-ci-correct-file-diff branch 2 times, most recently from eb7703e to 8da5d74 Compare January 4, 2024 00:50
@castastrophe castastrophe requested a review from mdt2 January 4, 2024 00:55
@castastrophe castastrophe added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. skip_vrt Add to a PR to skip running VRT (but still pass the action) github_actions Pull requests that update GitHub Actions code labels Jan 4, 2024
@castastrophe castastrophe force-pushed the chore-ci-correct-file-diff branch 4 times, most recently from cf904cd to 5af78b3 Compare January 4, 2024 01:37
castastrophe
castastrophe commented Jan 4, 2024
View reviewed changes
.vscode/settings.json
@@ -5,7 +5,7 @@
},
"[javascript]": {
"editor.codeActionsOnSave": {
"source.organizeImports": true
"source.organizeImports": "explicit"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This snuck in here but explicit is the default and is technically the same as true so it's not likely to cause any upset: https://code.visualstudio.com/docs/typescript/typescript-refactoring#_code-actions-on-save

mdt2
mdt2 reviewed Jan 4, 2024
View reviewed changes
Copy link
Collaborator

@mdt2 mdt2 left a comment

Choose a reason for hiding this comment

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

This is looking good to me, just one question!

.github/actions/file-diff/index.js Outdated
Comment on lines 103 to 109
if (changeSummary !== "") {
summary.push(...[
changeSummary,
"Table below reports only on changes to a package's main file. Other changes can be found in the collapsed \"Details\" below.",
""
]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not seeing this note in this PR or the storybook PR this was tested on 🤔 Am I missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah it looks like I added the note after I had merged & run this on the sub-branch. I just pulled in the latest so it's running now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool! I see it in the Storybook PR now 👍

@castastrophe castastrophe force-pushed the chore-ci-correct-file-diff branch from 5af78b3 to 2a21f98 Compare January 4, 2024 17:14
@castastrophe castastrophe merged commit a665a63 into main Jan 4, 2024
@castastrophe castastrophe deleted the chore-ci-correct-file-diff branch January 4, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdt2 mdt2 mdt2 approved these changes

Assignees
No one assigned
Labels
github_actions Pull requests that update GitHub Actions code size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@castastrophe @mdt2