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

Update readme.md #905

Closed
wants to merge 29 commits into from
Closed

Update readme.md #905

wants to merge 29 commits into from

Conversation

Zo-Bro-23
Copy link
Contributor

Updated README with more precise build instructions (#559)

@Zo-Bro-23
Copy link
Contributor Author

Also added a GitHub Actions workflow to build the app. I'm only familiar with Windows, so it only supports Windows at the moment, but feel free to add jobs for Mac and Linux too!

@Zo-Bro-23
Copy link
Contributor Author

Added Linux build support to the action too!

Copy link
Owner

@th-ch th-ch left a comment

Choose a reason for hiding this comment

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

Readme change looks good (apart from one line), the rest should not be needed, unless I missed something?

readme.md Outdated
```
1. Clone the repo
2. Run ```yarn```
3. Run ```npm i rimraf```
Copy link
Owner

Choose a reason for hiding this comment

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

rimraf is already a dependency so this step should not be needed!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rimraf is already a dependency so this step should not be needed!

Removed this line!

@@ -0,0 +1,127 @@
on:
Copy link
Owner

Choose a reason for hiding this comment

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

It seems this will do the same as the build workflow, no? (it already takes care of publishing the artifacts to GH releases)

@Zo-Bro-23
Copy link
Contributor Author

There seems to be an error regarding rimraf, which I wasn't able to quite pinpoint. Any build is failing, and removing rimraf from the build script seems to fix it. Sometimes, running npm i rimraf fixes the issue, but other times, it still persists. This issue is the reason for both the changes you mentioned. The GH Actions script I wrote doesn't use rimraf, because it's not necessary to delete the folder dist when running on Actions (because the folder doesn't exist). The build.yml workflow also has some other bugs, which means that it fails. Shall I replace the build.yml workflow with my own code from the release.yml workflow (Linux and Windows at least)?

Also, can you approve the change to package.json adding the two new scripts (build:winactions and build:linuxactions)? Those run without rimraf, so if you approve those, then I can remove the npm i rimraf line from the README.

@DaveThuet
Copy link

Hi...
I have an idea or suggestion for the app.
How about if there was a shortcut for liking and disliking the playback.
Best Regards
(With Google Translate from German to English)

@Zo-Bro-23
Copy link
Contributor Author

Hi... I have an idea or suggestion for the app. How about if there was a shortcut for liking and disliking the playback. Best Regards (With Google Translate from German to English)

Hi, this is not the place to post suggestions. I've reposted this for you in issues (#922). Please add further comments and discussions there. Thank you!

@th-ch th-ch mentioned this pull request Dec 26, 2022
@Zo-Bro-23
Copy link
Contributor Author

For the time being, if you don't approve of the changes to release.yml and package.json, can you cherrypick the changes to readme.md and merge only that? Thanks!

@Zo-Bro-23 Zo-Bro-23 closed this Jan 11, 2023
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.

3 participants