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

chore: simplify update script #94

Merged
merged 1 commit into from
Jul 23, 2023

Conversation

cmars
Copy link
Contributor

@cmars cmars commented Jul 22, 2023

anyproto-heart artifacts do not need github authentication to download. This might be left over from before this project was public. Anonymous downloading is safer for developers and preserves a bit more privacy.

Make "ubuntu-latest" the default platform. A bit opinionated, but linux is generally a safe default for CI and Docker, so might be reasonable. I can yoink this particular if it's too controversial.

Thanks!

@ra3orblade
Copy link
Contributor

You have changed update.sh but it's part of CI process and ubuntu-latest is a platform used by Github actions, so I can't accept this change unfortunately.

@cmars cmars force-pushed the chore/simplify-update-script branch 2 times, most recently from 13f8142 to b374ecb Compare July 22, 2023 17:06
@cmars
Copy link
Contributor Author

cmars commented Jul 22, 2023

You have changed update.sh but it's part of CI process and ubuntu-latest is a platform used by Github actions, so I can't accept this change unfortunately.

Ah, makes sense! I've reverted that bit. Thanks for reviewing!

anyproto-heart artifacts do not need github authentication to download.
This might be left over from before this project was public. Anonymous
downloading is safer for developers.

Make "ubuntu-latest" the default platform.
@cmars cmars force-pushed the chore/simplify-update-script branch from b374ecb to 772a479 Compare July 22, 2023 17:10
@cmars
Copy link
Contributor Author

cmars commented Jul 22, 2023

Updated github actions with the update.sh argument changes.

@negue
Copy link
Contributor

negue commented Jul 22, 2023

Oh nice - I also wanted to ask about the Auth not needed anymore in the update.sh 🎉

@cmars did you had no issues while trying to npm run start:dev ? I had to push the anytypeHelper into the dist folder first ( #92 ) - so I wonder if thats only an error on my side or if the update.sh needs another step to copy the binary into dist

@ra3orblade
Copy link
Contributor

Oh nice - I also wanted to ask about the Auth not needed anymore in the update.sh 🎉

@cmars did you had no issues while trying to npm run start:dev ? I had to push the anytypeHelper into the dist folder first ( #92 ) - so I wonder if thats only an error on my side or if the update.sh needs another step to copy the binary into dist

As it runs two times for macos and linux it basically creates two folders darwin-amd and darwin-arm which are then copied to dist by electronn hook. So no, update.sh doesnt need this step, if you want update.sh to work correctly you can just fix README.md, imo, which adds mv anytypeHelper step, do not forget that windows has exe file.

@ra3orblade
Copy link
Contributor

Updated github actions with the update.sh argument changes.

looks better, I will check once again and merge when I will be able to merge, right now my colleague is doing some licensing magic =)

update.sh Show resolved Hide resolved
@fuksman
Copy link
Member

fuksman commented Jul 23, 2023

recheck

@github-actions
Copy link

github-actions bot commented Jul 23, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@cmars
Copy link
Contributor Author

cmars commented Jul 23, 2023

I have read the CLA Document and I hereby sign the CLA

@ra3orblade ra3orblade merged commit 458530f into anyproto:main Jul 23, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants