Skip to content

Conversation

@xoascf
Copy link
Member

@xoascf xoascf commented Feb 14, 2023

  • Uses LF instead of CRLF
  • No spaces in empty lines
  • Simplified asset headers using an ALIGN_ASSET(x) macro

Build Artifacts

@briaguya0
Copy link
Contributor

@xoascf what happens when running ExtractAssets with these changes? My assumption is it leads to a giant git diff.

if we want to change these files we'll need to change ExtractAssets to generate them differently

@dcvz
Copy link
Contributor

dcvz commented Feb 15, 2023

Copy link
Contributor

@dcvz dcvz left a comment

Choose a reason for hiding this comment

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

Overall looking pretty good. Just one question here

@xoascf xoascf force-pushed the simplify-asset-headers branch 3 times, most recently from 5430d81 to 29f048d Compare February 16, 2023 20:41
@xoascf xoascf requested a review from dcvz February 16, 2023 21:21
@dcvz
Copy link
Contributor

dcvz commented Mar 2, 2023

@xoascf could you please do one more rebase before I merge this? I'm not sure if we have the latest headers (at some point we added also the MQ headers).

@xoascf
Copy link
Member Author

xoascf commented Mar 2, 2023

@xoascf could you please do one more rebase before I merge this? I'm not sure if we have the latest headers (at some point we added also the MQ headers).

Yes, shortly after that update, the headers were updated again for this PR, there should be no conflicts, but I will rebase again to make sure.

Update: There have been no changes after rerunning the ExtractAssets target for MQ.

@xoascf xoascf force-pushed the simplify-asset-headers branch from 29f048d to 324cc47 Compare March 2, 2023 14:18
Copy link
Contributor

@dcvz dcvz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@dcvz
Copy link
Contributor

dcvz commented Mar 2, 2023

Thanks for this @xoascf !

@dcvz dcvz merged commit d3f864c into HarbourMasters:develop Mar 2, 2023
@xoascf xoascf deleted the simplify-asset-headers branch March 2, 2023 14:47
leggettc18 pushed a commit to leggettc18/Shipwright that referenced this pull request Mar 10, 2023
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