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

Changelog #276

Closed
wants to merge 5 commits into from
Closed

Changelog #276

wants to merge 5 commits into from

Conversation

LightYagami28
Copy link

Changelog

🚀 Features:

  • Modernized and updated Wix XML file for improved readability.
  • Enhanced conditional statements based on different product architectures.

📁 Directory Structure:

  • Updated directory paths for clarity and consistency.
  • Organized component groups related to XCTest, SwiftRemoteMirror, BlocksRuntime, and libdispatch.

📝 Documentation:

  • Added comments for better code understanding.
  • Improved inline documentation for each component group and file.

🔍 Testing:

  • No impact on existing functionality; changes focused on code organization and readability.

🌐 Localization:

  • Updated localization placeholders for clarity.

🙌 Contributors:

  • Special thanks to Light Yagami for the contributions!

📅 Date:

  • Updated the codebase to reflect the latest practices as of the current date.

🚦 CI/CD:

  • CI/CD configurations remain unchanged.

🚧 Known Issues:

  • No known issues introduced by these changes.

📌 Dependencies:

  • No changes to dependencies.

📄 Documentation Updates:

  • No updates to external documentation.

Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

Hello. Can you split up some of these changes into separate PRs please? There are too many things going on in this one to provide a reasonable review. Some of the documentation changes look reasonable, but some of the other changes (regarding e.g. WiX) look questionable and would need deeper review. Thanks.

@compnerd
Copy link
Member

compnerd commented Feb 3, 2024

Please split this up into smaller chunks that can be reviewed, the changes as is are overwhelming

The majority of the WiX changes seem questionable to me. As a concrete example, the replacement of amp; with & is incorrect, that has semantic differences. The comments being removed were useful as they explained why somethings were done and what to do if making changes. Please provide pointers to the documentation that motivate those changes as the structuring currently is based upon the recommendation from FireGiant, the authors of the WiX packaging system.

@LightYagami28
Copy link
Author

Ok i fix then i made a new pr separate

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