Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

feat: use jsdeliver instead of github url #687

Merged
merged 42 commits into from
May 26, 2023
Merged

feat: use jsdeliver instead of github url #687

merged 42 commits into from
May 26, 2023

Conversation

0xMRTT
Copy link
Member

@0xMRTT 0xMRTT commented Jan 3, 2023

Description

We can use JSDeliver for downloading files instead of directly getting them from github because:

  • CDN all around the world (more than 750 point of presence) and in China
  • based on multiple providers
  • almost 100% uptime
  • files are stored in a permanent S3 storage to ensure that files remain avaiilble even if Github goes down.

Blocking

  • Maybe, that can lead to slow propagation of preset if we do changes

Type of change

  • Bugfix (Change which fixes a issue)
  • New feature (Change which adds new functionality)
  • Enhancement (Change which slightly improves existing code)
  • Breaking change (This change will introduce incompatibility with existing functionality)

Changelog

  • Changed repo url

Testing

  • I have tested my changes and verified that they work as expected

Signed-off-by: 0xMRTT <0xMRTT@proton.me>
since it's only doing build, we can call it build :)

Signed-off-by: 0xMRTT <0xMRTT@proton.me>

# Description

<!-- Describe your changes in detail here. -->

Fixes #(issue)

## Type of change

<!-- What type of change does your pull request introduce? Put an `x` in
the box that apply. -->
- [ ] Bugfix (Change which fixes a issue)
- [ ] New feature (Change which adds new functionality)
- [ ] Enhancement (Change which slightly improves existing code)
- [ ] Breaking change (This change will introduce incompatibility with
existing functionality)

## Changelog <!-- This is optional, but highly appreciated. -->

- Fixed …
- Added …

## Testing

- [ ] I have tested my changes and verified that they work as expected
<!-- Required, your PR won't be accepted if you don't do this step. -->

### How to test the changes

<!-- Optional, it can speed up review process if you provide the
information on how to test your changes. -->
No information provided.

Signed-off-by: 0xMRTT <0xMRTT@proton.me>
Signed-off-by: 0xMRTT <0xMRTT@proton.me>
@tfuxu
Copy link
Member

tfuxu commented Jan 3, 2023

Maybe we should inform users about using JSDeliver service? I don't think we need to do this in-app, but a small note section in readme would be good to add.

@daudix
Copy link
Member

daudix commented Jan 3, 2023

maybe this is over-kill for our app?

@0xMRTT
Copy link
Member Author

0xMRTT commented Jan 4, 2023

That can help to provide preset everytime, even if github goes down ...

@tfuxu
Copy link
Member

tfuxu commented Jan 4, 2023

The idea with JSDeliver is good, but we should implement it in such a way that it will be opt-in to use. Users could go for example to preferences and toggle an option to enable it when GitHub (for example) goes down.

@tfuxu tfuxu marked this pull request as draft January 6, 2023 00:18
@tfuxu
Copy link
Member

tfuxu commented Jan 6, 2023

Converted to draft, as we need to add this option to preferences window.

@0xMRTT 0xMRTT closed this May 6, 2023
@0xMRTT 0xMRTT deleted the jsdeliver branch May 6, 2023 12:23
@tfuxu
Copy link
Member

tfuxu commented May 6, 2023

Why?

@0xMRTT
Copy link
Member Author

0xMRTT commented May 6, 2023

I think that's not necessary ...

@0xMRTT 0xMRTT restored the jsdeliver branch May 6, 2023 12:44
@GradienceBot GradienceBot deleted the jsdeliver branch May 6, 2023 12:52
@tfuxu
Copy link
Member

tfuxu commented May 6, 2023

It isn't necessary, but it still would be useful if Github for example went down

@tfuxu tfuxu restored the jsdeliver branch May 6, 2023 13:18
@tfuxu tfuxu reopened this May 6, 2023
Ali-x98 and others added 7 commits May 23, 2023 20:13
Currently translated at 100.0% (292 of 292 strings)

Translation: Gradience/Gradience
Translate-URL: https://hosted.weblate.org/projects/GradienceTeam/gradience/ar/
Currently translated at 92.1% (269 of 292 strings)

Translation: Gradience/Gradience
Translate-URL: https://hosted.weblate.org/projects/GradienceTeam/gradience/nl/
Currently translated at 100.0% (292 of 292 strings)

Translation: Gradience/Gradience
Translate-URL: https://hosted.weblate.org/projects/GradienceTeam/gradience/ta/
Currently translated at 100.0% (292 of 292 strings)

Translation: Gradience/Gradience
Translate-URL: https://hosted.weblate.org/projects/GradienceTeam/gradience/nl/
@0xMRTT
Copy link
Member Author

0xMRTT commented May 23, 2023

@daudix-UFO RFC for title and subtitle

@tfuxu Request for code review

@daudix
Copy link
Member

daudix commented May 23, 2023

First issue, can't start

Приложение запущено в 22:02:26
                              Traceback (most recent call last):
  File "/app/bin/gradience", line 70, in <module>
    from gradience.frontend import main
  File "/app/lib/python3.10/site-packages/gradience/frontend/main.py", line 27, in <module>
    from gradience.backend.globals import presets_dir, get_gtk_theme_dir
  File "/app/lib/python3.10/site-packages/gradience/backend/globals.py", line 53, in <module>
    preset_repos = preset_repos_github
NameError: name 'preset_repos_github' is not defined
Приложение завершилось

@0xMRTT 0xMRTT marked this pull request as ready for review May 23, 2023 19:41
@0xMRTT
Copy link
Member Author

0xMRTT commented May 23, 2023

Oh messed up when resolving the merge conflict 🤫

@tfuxu
Copy link
Member

tfuxu commented May 23, 2023

I'll review the code tomorrow, as it's pretty late where I live, but from what I quickly tested, it looks and works fine

Copy link
Member

@tfuxu tfuxu left a comment

Choose a reason for hiding this comment

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

Works for me after a fix. I'd like to re-request @0xMRTT and @daudix-UFO for review to make sure it works properly on your machines too.

@tfuxu tfuxu requested a review from daudix May 25, 2023 19:03
Copy link
Member

@daudix daudix left a comment

Choose a reason for hiding this comment

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

Works fine :)

@0xMRTT
Copy link
Member Author

0xMRTT commented May 25, 2023

Hmm, I think that there is one preset repos which is still using the hardcoded one but idk where it is since I'm not at home...

@daudix
Copy link
Member

daudix commented May 25, 2023

JSDeliver is used for both official and curated, so I think it's fine

image

@0xMRTT
Copy link
Member Author

0xMRTT commented May 26, 2023

Tried, and that's okay for me!

@0xMRTT 0xMRTT merged commit 3280781 into main May 26, 2023
5 checks passed
@0xMRTT 0xMRTT deleted the jsdeliver branch May 26, 2023 06:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants