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

Switch from m-scr-p and m-bundle-p to bnd-m-p #2761

Merged
merged 8 commits into from
Jan 21, 2022

Conversation

kwin
Copy link
Contributor

@kwin kwin commented Dec 14, 2021

This closes #1718

entry = zis.getNextEntry();
}
}
// metatype descriptors must come last (after component descriptions)
Copy link
Contributor Author

@kwin kwin Dec 15, 2021

Choose a reason for hiding this comment

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

@justinedelson Any reason why you think that metatype attributes should overwrite the value of same named component properties? IMHO those two are not directly related. Currently it feels a bit like we a comparing apples and pears. The correct way would be to compare SCR descriptors and metatype descriptors separately, as one affects the default properties of DS components, the other one only how the Felix Web Console renders a UI for setting custom properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may easily lead to false positives e.g. in case you change the component property to another type while keeping the metatype with the previous type.

@codecov
Copy link

codecov bot commented Dec 15, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@5ceb9eb). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2761   +/-   ##
=========================================
  Coverage          ?   53.79%           
  Complexity        ?     5528           
=========================================
  Files             ?      752           
  Lines             ?    30455           
  Branches          ?     3936           
=========================================
  Hits              ?    16383           
  Misses            ?    12545           
  Partials          ?     1527           
Impacted Files Coverage Δ
...ava/com/adobe/acs/commons/util/impl/Activator.java 82.35% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ceb9eb...d6e43cf. Read the comment docs.

@kwin kwin force-pushed the feature/bnd-maven-plugin branch 2 times, most recently from 7c88e0d to 31f954c Compare December 15, 2021 09:57
@kwin kwin marked this pull request as ready for review January 10, 2022 15:24
@kwin
Copy link
Contributor Author

kwin commented Jan 20, 2022

@davidjgonzalez Can you review? I would like to merge soon...

@davidjgonzalez davidjgonzalez merged commit 37e385c into master Jan 21, 2022
@davidjgonzalez davidjgonzalez deleted the feature/bnd-maven-plugin branch January 21, 2022 15:30
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.

Use bnd-maven-plugin instead of maven-bundle-plugin
2 participants