search

jelhan / ember-style-modifier

{{style}} element modifier for ember.js
MIT License
36 stars 12 forks source link

Convert addon to V2 #218

Closed mkszepp closed 8 months ago

mkszepp commented 9 months ago

Fix #217

mkszepp commented 9 months ago

@jelhan i think this is ready... please approve workflow, so we can check of all tests do pass

jelhan commented 9 months ago

Thanks a lot for working on picking this up that quickly!

How did you converted it? Is it based on Embroider addon blueprints?

I noticed some misalignments with Ember CLI conventions. E.g. Prettier configuration is different. If this mismatch is caused by the blueprints, I would report upstream rather than discussing in this PR. That's why I'm asking.

mkszepp commented 9 months ago

@jelhan for v2 addon migration i have always used https://github.com/NullVoxPopuli/ember-addon-migrator this is using embroider blueprint internal

The migrator has some problems, like readme, changelog or renovate will not be readded correctly. Also docs will be lost completly... This is a manual step for the moment. Also while migration i have got errors by porting other addons (this one was going very simple). This is atm a beta, but it is helpful.

The migrator make +- new apps and is readding all files from before.

I will check the prettier file... because i think he has hold the standard files, not the file from repo

Edit: i have now readded some settings... i think we have now everything or?

jelhan commented 9 months ago

Thanks a lot for providing the details. If changes are due to different opinions of Ember CLI and Embroider blueprints that's fine for me and nothing which needs to be changed here. I will report those upstream and of there is a decision to align, it should propagate to this repo in next Ember CLI Upgrade.

Prettier is one example. Ember CLI configures Prettier to use double quotes as standard. And only overwrites for JavaScript and TypeScript files to use single quotes instead. (Source) Embroider addon blueprints configure Prettier to use single quotes for all files. (Source) For this PR both is fine for me. But I think long-term it should be aligned between the blueprints.

I hope I have some time doing a full review this weekend. Otherwise I will get back to it early next week.

mkszepp commented 8 months ago

@jelhan did you have time for review?

mkszepp commented 8 months ago

@jelhan please don't merge renotate bot changes atm... because its not possible to merge this changes. i have now updated manually the merged dependency updates

mkszepp commented 8 months ago

@jelhan hope the test do pass now, otherwise let me know

jelhan commented 8 months ago

Released as v4.2.0.