Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixes issue #234 #238

Merged
merged 1 commit into from
May 1, 2023
Merged

Fixes issue #234 #238

merged 1 commit into from
May 1, 2023

Conversation

MakisChristou
Copy link
Contributor

@MakisChristou MakisChristou commented Apr 30, 2023

I have added a slight modification to the install.sh to use the rust bin directory that is already created and added to the path automatically on Windows.

Closes #234

@Autoparallel
Copy link
Collaborator

I don't have a Windows machine to test this on personally. @0xEstelle or @Alexangelj can you give this a shot?

@Autoparallel Autoparallel self-requested a review April 30, 2023 20:17
@Autoparallel
Copy link
Collaborator

Autoparallel commented Apr 30, 2023

I have added a slight modification to the install.sh to use the rust bin directory that is already created and added to the path automatically on Windows.

Hi @MakisChristou, I'm curious what you think about a slightly different approach. With cargo, we can instead do:

cargo install --path crates/cli --force

This will install the binary in
Users/$USER/.cargo/bin/arbiter
at least on MacOS.

From my experience, this also generates a symlink to arbiter.

This can still be put in an .install.sh script to make life a bit easier.

What do you think?

@MakisChristou
Copy link
Contributor Author

MakisChristou commented Apr 30, 2023

Hello @Autoparallel. I have tried this (cargo install --path crates/cli --force) on both MacOs and Windows 11 and it looks like its working. So maybe we can just skip the install.sh script altogether? I don't see why its needed.

@Autoparallel
Copy link
Collaborator

Hello @Autoparallel. I have tried this (cargo install --path crates/cli --force) on both MacOs and Windows 11 and it looks like its working. So maybe we can just skip the install.sh script altogether? I don't see why its needed.

Seems reasonable. Perhaps a good way to give you credit through this work is to edit the README with these instructions?

@MakisChristou
Copy link
Contributor Author

What about now?

@Autoparallel
Copy link
Collaborator

Perfect. I'll merge now.

Copy link
Collaborator

@Autoparallel Autoparallel left a comment

Choose a reason for hiding this comment

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

lgtm

@Autoparallel Autoparallel merged commit 2a8134d into anthias-labs:main May 1, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make an install script
2 participants