Skip to content

Conversation

@Eeems
Copy link

@Eeems Eeems commented Feb 10, 2023

I removed the precompiled binaries since it's not really best practice to include them in the source. Instead I'd recommend uploading a zip containing them and install.sh to a release tag.

@Eeems
Copy link
Author

Eeems commented Feb 10, 2023

Fixes #13

@Eeems
Copy link
Author

Eeems commented Feb 19, 2023

@funkey is there anything you'd like me to change before merging?

@funkey
Copy link
Owner

funkey commented Feb 21, 2023

Hi Nathaniel,

Thanks a lot for the PR. Adding RM1 support is great, happy to include that.

However, I don't want to remove the precompiled binaries. The reason I include them along the source is that not every user of recept might be able (or willing) to compile the code themselves. This will pose a barrier for most users.

So I suggest you either keep the precompiled binaries (they are tiny) or add a script to download them as you suggested. I'd very much prefer the former.

@Eeems
Copy link
Author

Eeems commented Feb 21, 2023

Let me clarify, I wasn't suggesting a script to download them, I was suggesting adding an artifact to a release tag that users can download that contains the existing install.sh script, as well as the precompiled binaries.

I wouldn't mind taking some time to automate this with github actions if you'd like.

@funkey
Copy link
Owner

funkey commented Feb 24, 2023

I see. Sure, go ahead then, sounds good to me!

@Eeems Eeems marked this pull request as draft May 4, 2023 20:01
@Eeems Eeems marked this pull request as ready for review May 4, 2023 20:30
@Eeems
Copy link
Author

Eeems commented May 4, 2023

Sorry for how long this took. I had some stuff come up. I've only automated the build here, but you should be able to automate creating a new version on merge to master, and attaching the build artifacts.

@Eeems
Copy link
Author

Eeems commented May 4, 2023

Example run since you have to allow my pull request to run this: https://github.com/Eeems/recept/actions/runs/4887019353

@Eeems
Copy link
Author

Eeems commented Nov 26, 2023

@funkey poke?

@Eeems
Copy link
Author

Eeems commented Jan 11, 2024

@funkey poke again?

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