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: Generative variants #47

Closed

Conversation

barelyhuman
Copy link
Contributor

@barelyhuman barelyhuman commented Aug 30, 2022

  • Removes the extension since the complexity isn't needed anymore
  • Add and remove needed details from the package.json
  • script to generate the variants with differences in props and file paths

Need confirmation on the following

  • Do we wish to keep the extension in place ?
  • Do we need the CI / Github action to build during a publish a step
  • Do we need the PR actions to just test if the generation works or do we need the generated files in the PR to actually be usable in a plugin?

Fix: #46

@miguelsolorio
Copy link
Owner

miguelsolorio commented Aug 30, 2022

Do we wish to keep the extension in place ?

Not sure what you mean by this?

Do we need the CI / Github action to build during a publish a step

I was thinking we could have this as a publish step that happens locally, what are your thoughts?

Do we need the PR actions to just test if the generation works or do we need the generated files in the PR to actually be usable in a plugin?

We could have a test in a PR action to ensure nothing breaks

Also: I need to test this to see if this new extension will work on the web/remote so let me know when you think this is ready and I can start testing it. Some docs on web extensions.

@barelyhuman
Copy link
Contributor Author

barelyhuman commented Aug 30, 2022

Do we wish to keep the extension in place ?

Not sure what you mean by this?

If you see the commits, the first one get's rid of all the JS logic , do we want to keep the logic?
if yes, I'll revert that commit (doesn't make the release a breaking one)
if not, then we just move ahead (makes the release a breaking one)

I was thinking we could have this as a publish step that happens locally, what are your thoughts?

Either would be fine, it's just a single JS file that needs to be executed

We could have a test in a PR action to ensure nothing breaks

Understood.

Also: I need to test this to see if this new extension will work on the web/remote so let me know when you think this is ready and I can start testing it. Some docs on web extensions

Shouldn't need to modify anything, you can go ahead and test it, just run this in the shell

./scripts/generate.js

and add the variants folder to the publish for now

@miguelsolorio
Copy link
Owner

Given that this is a stale PR and there hasn't been too much interest in support for the web, I will close this for now and am happy to revisit it in the future. Thanks so much for your suggestions ❤️ 👏

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.

Allow icon theme to run on the web and remote
2 participants