-
Notifications
You must be signed in to change notification settings - Fork 132
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
[server] Send missing attributes in ExtensionQueryResult #326
Conversation
It fixes #320 as well |
Remove --web option for publishing web extension will be inferred from vsixmanifest
cae2363
to
1c6a651
Compare
@spoenemann could you review please? cc @akosyakov |
server/build.gradle
Outdated
implementation 'com.fasterxml.jackson.core:jackson-databind:2.12.5' | ||
implementation 'com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.12.5' | ||
implementation 'com.fasterxml.woodstox:woodstox-core:6.2.4' | ||
implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-xml:2.12.5' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jackson is already included with spring-boot-starter-json
. Is it really necessary to declare the dependency here? If yes, please extract the common version to the versions
map.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah but I need a newer version as the included one is old and has a bug when parsing arrays in xml, is there a better way to configure for using a newer version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. Let's keep this in mind for later.
queryExt.publisher.publisherId = namespace.getPublicId(); | ||
queryExt.publisher.publisherName = namespace.getName(); | ||
queryExt.tags = latest.getTags(); | ||
// TODO: add these |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to file an issue for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I created #330
It seems this change had unintended side effects: #343 |
--web
from cli, web extensions are inferred by parsingextension.vsixmanifest
fileFixes #322
Fixes #320