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

🔀[build] Port build script to windows #2865

Merged
merged 40 commits into from
Feb 29, 2024
Merged

🔀[build] Port build script to windows #2865

merged 40 commits into from
Feb 29, 2024

Conversation

Cr4zyc4k3
Copy link
Collaborator

@Cr4zyc4k3 Cr4zyc4k3 commented Feb 1, 2024

What kind of update does this PR provide? Please check

  • new translations / updated translations / translation fixes
  • a new feature
  • a new module
  • a bugfix for an existing feature / module
  • improvement of style or user experience
  • other: Please fill out

What is included in your update?

@Cr4zyc4k3 Cr4zyc4k3 linked an issue Feb 1, 2024 that may be closed by this pull request
@Cr4zyc4k3 Cr4zyc4k3 added the enhancement New feature or request label Feb 1, 2024
@Cr4zyc4k3
Copy link
Collaborator Author

@deoxis9001 It now works on powershell in a default powershell on my system 🙈
Fyi: Currently the --port command is not working so you need to adjust it in the script if you want to use another port than 36551

@deoxis9001
Copy link
Contributor

image
image
is it possible to delete these messages locally?

@Cr4zyc4k3
Copy link
Collaborator Author

Cr4zyc4k3 commented Feb 4, 2024

Probably yes, but the effort would be much higher than the result.

The quick&dirty solution:
For the branches should be an empty branches.json file in the dist folder enough to remove this error.
For the release notes is a deeper inspection required.

@jxn-30 Thoughts on this topic?

@jxn-30
Copy link
Member

jxn-30 commented Feb 4, 2024

Probably yes, but the effort would be much higher than the result.

The quick&dirty solution: For the branches should be an empty branches.json file in the dist folder enough to remove this error. For the release notes is a deeper inspection required.

@jxn-30 Thoughts on this topic?

empty branches.json seems fine to me, maybe we could also provide empty Releasenote-Files or otherwise download them similar to the mission files in the prebuild step.

docs/en_GB/contributing/runningLocally.md Outdated Show resolved Hide resolved
docs/en_GB/contributing~dev Outdated Show resolved Hide resolved
docs/en_US/contributing/committing.md Show resolved Hide resolved
docs/en_US/contributing/runningLocally.md Outdated Show resolved Hide resolved
scripts/buildUserscript.ts Show resolved Hide resolved
@deoxis9001
Copy link
Contributor

image
I have other errors

@kdev
Copy link
Member

kdev commented Feb 13, 2024

Bun introduced a cross-os shell layer to.
Maybe it's worth a thought rewriting the build script in JS/TS with some sort of compile layer to make it run on multiple OS.
This would require a complete rewrite of the build script but would help maintainability a lot (something this project needs!).
Any thoughts?

Edit: Bun is not the first to introduce something like this. Google open-sourced a similar project years ago.

@jxn-30
Copy link
Member

jxn-30 commented Feb 13, 2024

Bun introduced a cross-os shell layer to. Maybe it's worth a thought rewriting the build script in JS/TS with some sort of compile layer to make it run on multiple OS. This would require a complete rewrite of the build script but would help maintainability a lot (something this project needs!). Any thoughts?

Edit: Bun is not the first to introduce something like this. Google open-sourced a similar project years ago.

I like the idea. That may also be useful for #2884 where currently weird child processes are spawned instead.

@Cr4zyc4k3
Copy link
Collaborator Author

Since google is famous for throwing project under the bus (https://killedbygoogle.com/) I won't relay on something they maintain. But I like the idea in general. Until this is done we should keep both versions.

@kdev
Copy link
Member

kdev commented Feb 13, 2024

Since google is famous for throwing project under the bus (https://killedbygoogle.com/) I won't relay on something they maintain. But I like the idea in general. Until this is done we should keep both versions.

Luckily they state that it's not a "supported Google product".
I guess they won't throw it under the bus since they are probably using it themselves (also the project is at v7 and has been around for a while now)
Anyways Google's zx is made for just linux scripts so it won't fit our use case.

@Cr4zyc4k3 what do you think about the Bun project?

@Cr4zyc4k3
Copy link
Collaborator Author

If it is possible to accomplish this without installing bun globally I'm cool with this solution.

@Cr4zyc4k3
Copy link
Collaborator Author

image image is it possible to delete these messages locally?

Should be fixed with 16c51fd

@Cr4zyc4k3 Cr4zyc4k3 requested a review from jxn-30 February 29, 2024 15:27
@Cr4zyc4k3 Cr4zyc4k3 self-assigned this Feb 29, 2024
@jxn-30 jxn-30 merged commit 73027c4 into dev Feb 29, 2024
8 checks passed
@jxn-30 jxn-30 deleted the feat/buildOnWindows branch February 29, 2024 15:53
@kdev
Copy link
Member

kdev commented Apr 16, 2024

Bun now supports windows. Maybe we should consider switching/rewriting for better maintainability?

@jxn-30
Copy link
Member

jxn-30 commented Apr 16, 2024

I think at that point we should take the chance and have a look at the whole CI and see where we could improve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request]: [dev] Build script for windows
4 participants