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

Fixed two deprecations warning, about "path" attribute type and default "target-property" in Marko Taglib #23

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

DanielVip3
Copy link

@DanielVip3 DanielVip3 commented Aug 25, 2020

Marko Taglib deprecated the attribute type which was "path".
Now it's standardized to use "string" also for paths.
I just changed the "path" types to "string" ones.
There also was another deprecation warning which was about "*" attributes: the default "target-property" will not be "*" anymore but null in the future, so it needs to explicitly be declared. I did so, in the two "*" attributes.

The deprecation warning doesn't appear again.
Everything works fine.

Marko Taglib deprecated the attribute type which was "path".
Now it's standardized to use "string" also for paths.

The deprecation warning doesn't appear again.
I just made a small mistake and forgot to change package-path's type too.

Also, this caused another warning which was about default "target-property" for "*" attributes.
I just put "target-property": "*" for the two "*" attributes in the json.

Deprecations warning are no more, yay!
@bruce808
Copy link

Is there a change missing? Still getting the target-property warning.

@DanielVip3
Copy link
Author

Sadly they are! I forgot a commit. I'm gonna commit again soon tomorrow to fix that.

Again, I just forgot to do this in the last commit. Whoops.
@DanielVip3
Copy link
Author

@bruce808 fixed.

@yomed
Copy link

yomed commented Aug 30, 2020

cc @mlrawlings @DylanPiercey Would be great to get this merged!

@DanielVip3
Copy link
Author

That would be cool, but this lib doesn't seem in active development anymore.
I suggest installing this forked branch directly from GitHub using npm, I'll try to keep them in sync whenever a (rare) commit comes for this repo.

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