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

App Submission: cobalt #2174

Merged
merged 37 commits into from
Mar 8, 2025
Merged

App Submission: cobalt #2174

merged 37 commits into from
Mar 8, 2025

Conversation

dennysubke
Copy link
Contributor

App Submission

cobalt

...

Icon

logo
...

Gallery images

1

2

3

4

5

...

I have tested my app on:

  • umbrelOS on a Raspberry Pi
  • umbrelOS on an Umbrel Home
  • umbrelOS on Linux VM

@dennysubke
Copy link
Contributor Author

dennysubke commented Feb 9, 2025

Hey @nmfretz

Just for your information: The developers of Cobalt didn't provide a Docker image with UI support, so I built one myself. The app is truly amazing, and I’d love to see this PR merged. 🚀

Port mapping to 9000 is needed. External access is required to this port because it is not proxied by the app_proxy.

Thanks for reviewing!

al-lac
al-lac previously requested changes Feb 17, 2025
Copy link
Collaborator

@al-lac al-lac left a comment

Choose a reason for hiding this comment

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

Hey @dennysubke! Another great submission! :octocat:

Added some points that should be changed. Also I think you can remove the directory as the cookies.json would not make a lot of sense now.

Tested it without it and everything worked just fine.

manifestVersion: 1
id: cobalt
name: cobalt
tagline: A media downloader that doesn't piss you off
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the tagline from their repo.

Suggested change
tagline: A media downloader that doesn't piss you off
tagline: Best way to save what you love

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I knew it wouldn't go through! 😂

Copy link
Contributor

Choose a reason for hiding this comment

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

ahaha

@dennysubke
Copy link
Contributor Author

dennysubke commented Feb 17, 2025

Hey @dennysubke! Another great submission! :octocat:

Added some points that should be changed. Also I think you can remove the directory as the cookies.json would not make a lot of sense now.

Tested it without it and everything worked just fine.

All changes have been made now! Thanks again @al-lac for your help and feedback! 😊

@nmfretz
Copy link
Contributor

nmfretz commented Mar 3, 2025

Excellent, thanks @dennysubke. Great initiative creating the image 👌. And thanks for reviewing @al-lac.

I've changed the API to the nearest free port (9013). We'll make the equivalent change in the gallery image. Great idea including the important config instruction as a gallery asset.

Something to consider: how do you feel about moving the note about the processing server config higher in the app description? This way it's not hidden until the user clicks "read more" in the app store. If the user doesn't set this then they'll be using cobalt's server instead which may not be clear to the average user.

If we don't want a ⚠️ symbol to be the first thing shown in the app description we could just use a settings cog ⚙️ or tool icon 🛠️ instead. Something like:

⚙️ In order use your own cobalt instance, go to settings > instances > and toggle "use a custom processing server". Add http://umbrel.local:9013 as your server.

@dennysubke
Copy link
Contributor Author

Excellent, thanks @dennysubke. Great initiative creating the image 👌. And thanks for reviewing @al-lac.

I've changed the API to the nearest free port (9013). We'll make the equivalent change in the gallery image. Great idea including the important config instruction as a gallery asset.

Something to consider: how do you feel about moving the note about the processing server config higher in the app description? This way it's not hidden until the user clicks "read more" in the app store. If the user doesn't set this then they'll be using cobalt's server instead which may not be clear to the average user.

If we don't want a ⚠️ symbol to be the first thing shown in the app description we could just use a settings cog ⚙️ or tool icon 🛠️ instead. Something like:

⚙️ In order use your own cobalt instance, go to settings > instances > and toggle "use a custom processing server". Add http://umbrel.local:9013 as your server.

Thanks for the feedback and suggestions @nmfretz! I agree with moving the note higher in the app description to make it more visible. Also, I think using a settings icon ⚙️ instead of the ⚠️ symbol is a great idea for a cleaner look. I've made the necessary adjustments.

@nmfretz nmfretz dismissed al-lac’s stale review March 8, 2025 04:15

Requested changes completed

Copy link

github-actions bot commented Mar 8, 2025

⚠️   Linting finished with 1 warning   ⚠️

Thank you for your submission! This is an automated linter that checks for common issues in pull requests to the Umbrel App Store.

Please review the linting results below and make any necessary changes to your submission.

Linting Results

Severity File Description
ℹ️ cobalt/docker-compose.yml External port mapping "9013:9000":
Port mappings may be unnecessary for the app to function correctly. Docker's internal DNS resolves container names to IP addresses within the same network. External access to the web interface is handled by the app_proxy container. Port mappings are only needed if external access is required to a port not proxied by the app_proxy, or if an app needs to expose multiple ports for its functionality (e.g., DHCP, DNS, P2P, etc.).
⚠️ cobalt/umbrel-app.yml "icon" and "gallery" needs to be empty for new app submissions:
The "icon" and "gallery" fields must be empty for new app submissions as it is being created by the Umbrel team.

Legend

Symbol Description
Error: This must be resolved before this PR can be merged.
⚠️ Warning: This is highly encouraged to be resolved, but is not strictly mandatory.
ℹ️ Info: This is just for your information.

@nmfretz
Copy link
Contributor

nmfretz commented Mar 8, 2025

Thanks again @dennysubke. I'm sending this live now with 4 of the gallery assets and then will add the 5th one with port 9013 instead of 9000 soon.

image

@nmfretz nmfretz merged commit a26c8c5 into getumbrel:master Mar 8, 2025
1 check passed
@dennysubke
Copy link
Contributor Author

Thanks again @dennysubke. I'm sending this live now with 4 of the gallery assets and then will add the 5th one with port 9013 instead of 9000 soon.

image

Thanks @nmfretz! Do you want me to take care of adding the 5th gallery asset with port 9013, or do you already have it covered?

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