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

feat(snippets): add more rounded corners #478

Merged
merged 4 commits into from
Apr 13, 2023
Merged

feat(snippets): add more rounded corners #478

merged 4 commits into from
Apr 13, 2023

Conversation

Fadexz
Copy link
Contributor

@Fadexz Fadexz commented Apr 10, 2023

Adds some changes to round some extra elements.

Added rounded images snippet + snippet image
Adds changes to rounded corners snippet mostly adding some more elements to round.
@Fadexz Fadexz requested a review from a team as a code owner April 10, 2023 09:56
@Fadexz Fadexz requested review from rxri and removed request for a team April 10, 2023 09:56
Copy link
Member

@theRealPadster theRealPadster left a comment

Choose a reason for hiding this comment

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

@Fadexz It looks like there were merge conflicts. I think your branch may not be up to date with upstream, since the "Rounded Images" snippet was added. Can you just double check that I merged it properly and the snippet looks as intended? We can merge once you approve.

Copy link
Contributor Author

@Fadexz Fadexz left a comment

Choose a reason for hiding this comment

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

Looks good

@Fadexz
Copy link
Contributor Author

Fadexz commented Apr 12, 2023

Seems for some reason it wasn't in there from before, so there was a conflict it seems.
Does this only take effect when the marketplace has an update and isn't fetched automatically?

@rxri rxri changed the title feat: Rounded Images Snippet Update chore: rou Images Snippet Apr 12, 2023
@rxri rxri changed the title chore: rou Images Snippet chore(snippets): add more rounded corners Apr 12, 2023
@rxri rxri changed the title chore(snippets): add more rounded corners feat(snippets): add more rounded corners Apr 12, 2023
@theRealPadster
Copy link
Member

Seems for some reason it wasn't in there from before, so there was a conflict it seems. Does this only take effect when the marketplace has an update and isn't fetched automatically?

Yeah I think it was because you had an existing fork that was behind upstream. You need to sync your fork so it's up to date or it will be behind and could have merge conflicts.

@theRealPadster theRealPadster merged commit c5b352f into spicetify:main Apr 13, 2023
@Fadexz
Copy link
Contributor Author

Fadexz commented Apr 14, 2023

Seems for some reason it wasn't in there from before, so there was a conflict it seems. Does this only take effect when the marketplace has an update and isn't fetched automatically?

Yeah I think it was because you had an existing fork that was behind upstream. You need to sync your fork so it's up to date or it will be behind and could have merge conflicts.

Yes I tried to do that at the time but it seems it didn't work so I just deleted the local copy to make sure it was all good.

If you don't mind, do you know the answer to my question? Do snippets only get added to the marketplace once a new release of the marketplace occurs? I assume that it is since I don't see it, so it would take maybe a month or so?

I think this answers my question though: #435

@theRealPadster
Copy link
Member

theRealPadster commented Apr 14, 2023

If you don't mind, do you know the answer to my question? Do snippets only get added to the marketplace once a new release of the marketplace occurs? I assume that it is since I don't see it, so it would take maybe a month or so?

I think this answers my question though: #435

If I recall correctly, it fetches and caches the snippets from the GitHub repo. So doesn't need a new release, just needs to be merged in. If you're not seeing it, you may need to restart your Spotify. Though if the issue is that a snippet changed but the name is the same, it might still be cached, in which case you might want to reset marketplace or remove the cached snippets from localstorage manually.

That issue is different. It's proposing we let users put snippets in a manifest file and host them in their own repo, like how the rest of the marketplace works.

@Fadexz
Copy link
Contributor Author

Fadexz commented Apr 14, 2023

If I recall correctly, it fetches and caches the snippets from the GitHub repo. So doesn't need a new release, just needs to be merged in. That issue is different. It's proposing we let users put snippets in a manifest file and host them in their own repo, like how the rest of the marketplace works.

I see. I have tried to do that before and have just now to make sure and I even see the lists populate but I don't see the snippet I added. So you can see it just to clarify? If so i'll have to try a bit harder to make sure I clear everything.

@theRealPadster
Copy link
Member

I have it in my snippets, but it looks like it's the version that hasn't been updated. I took another look at the code and it looks like it's actually getting them from the local file (your local copy of the marketplace custom app) and not pulling from github. So you'd have to download the latest build of marketplace to update it. We should probably change it to do it remotely, if you want to make a new issue for that.

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