Skip to content

Code Quality: Ignored Files.App.Launcher.exe.sha256 #17075

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Lamparter
Copy link
Contributor

Correctly ignores the Files.App.Launcher.exe.sha256 file that was not correctly ignored previously.

Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's ignored it will get deleted.

Copy link
Member

@yaira2 yaira2 Apr 29, 2025

Choose a reason for hiding this comment

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

It'll be removed from the repo, or it just won't be included when the repo is cloned?

Copy link
Member

Choose a reason for hiding this comment

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

TBH theres no reason to keep this file in the repo or on one's local because everytime you run CD, it'll be overridden and either:

  • The CD should embed the SHA to the files.exe binary.
  • The CD should output the SHA and you should copy&paste to the corresponding release blog.

CC: @hishitetsu I believe you added because we or users better can check the SHA of the binary right?

Copy link
Member

@yaira2 yaira2 Apr 29, 2025

Choose a reason for hiding this comment

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

because everytime you run CD

Originally the file was only updated when the launcher project was modified, but we lost this behavior with one of our other changes. I'm hoping we can fix the build step at some point, but it's low priority since it doesn't have any impact.

Copy link
Member

Choose a reason for hiding this comment

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

The important thing is that we should keep the SHA so users can compare on their local but in the first place we have never done so (if Hishitetsu meant so).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'll be removed from the repo, or it just won't be included when the repo is cloned?

If it's in the ignore but still in the repository Git will let you track changes to it.

Copy link
Member

Choose a reason for hiding this comment

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

The hash value is provided to see if the locally deployed FilesLauncher.exe needs to be updated.
If the hash is not correct, FilesLauncher.exe will not be properly updated.
#11750

@yaira2 yaira2 added the changes requested Changes are needed for this pull request label Apr 28, 2025
@yaira2 yaira2 changed the title Code Quality: Correctly ignored Files.App.Launcher.exe.sha256 Code Quality: Ignored Files.App.Launcher.exe.sha256 Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes requested Changes are needed for this pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants