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

Simplified build, replaced ziget dependency with std.http and added option to download zls #98

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

FabricatorZayac
Copy link

No description provided.

@FabricatorZayac
Copy link
Author

I also added a zigup-init.sh script that downloads and installs the latest version of zigup

@FabricatorZayac
Copy link
Author

I originally just wanted to just add a zls downloader (kinda like rustup component add rust-analyzer), but then I bumped into an issue with ziget erroring instead of redirecting on code 302 Found. So I replaced it with std.http and it worked

@marler8997
Copy link
Owner

Wow you've done alot here!

Note that there's currently an intermittent issue around TLS with std.http which is why I haven't switched to it yet (see #75).

Really like that you've made a zigup-init.sh file, that's been on the TODO list for a while. I don't know enough about ZLS and it's use cases to know whether zigup should also be managing it, I'll have to look into that more. I might go through this PR and cherry-pick the commits I can take right away and then maybe we can look into the other parts more.

@marler8997
Copy link
Owner

Here's how I'd like to see these changes organized. First, let's get "zig fmt" out of the way (#99 done). Here's a list of PR's I would split this into:

  • PR: Misc Changes
    • Fix MisInfo
    • Add Manual Dispatch
    • remove deprecated names (std.build.Builder with std.Build)
  • PR: Reformat --help
  • PR: Add ZLS Installation
  • PR: Add zigup-init script

There is already a PR for switching to std.http (#75). If you feel like it's too much work to organize your changes in this way not a big deal, I can cherry-pick your commits and rebase and do the magic git foo myself, but, I thought I'd let you know what I'd like to see if you're up for it.

@FabricatorZayac
Copy link
Author

FabricatorZayac commented Sep 7, 2023

I'm not super experienced with github PRs, so I am not exactly sure how to split up my changes into multiple PRs. I've only done very minor contributions in the past.

I am just going to look up how to do that

@FabricatorZayac
Copy link
Author

As I understand, to do that I have to branch from the old master and cherry pick commits into the new branches and then make PRs from those?

@FabricatorZayac
Copy link
Author

One problem I am running into trying to cherry-pick the "remove deprecated names" commits is that I did that alongside removing the ziget dependency and removing the first build step that fetched it. It should probably be separate from other misc changes

@FabricatorZayac
Copy link
Author

ZLS installation didn't work with ziget, so that can only be added on top of migrating to std.http.
The only change I can fully separate is the zigup-init.sh script

@FabricatorZayac
Copy link
Author

Created the zigup-init pr (#100)

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.

2 participants