Skip to content

JCRVLT-336 - adding support for configuring installationScope for FSP…#43

Closed
DominikSuess wants to merge 3 commits into
apache:trunkfrom
DominikSuess:issue/JCRVLT-336
Closed

JCRVLT-336 - adding support for configuring installationScope for FSP…#43
DominikSuess wants to merge 3 commits into
apache:trunkfrom
DominikSuess:issue/JCRVLT-336

Conversation

@DominikSuess
Copy link
Copy Markdown
Contributor

…ackageRegistry

* @param scope to set a corresponding workspacefilter
* @throws IOException If an I/O error occurs.
*/
public FSPackageRegistry(@Nonnull File homeDir, String scope) throws IOException {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not use the InstallationScope class ?

*/
package org.apache.jackrabbit.vault.packaging.registry.impl;

public final class InstallationScope {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wouldn't a enum be better ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tripodsan - actually I started with it but the problem is that there is no static access to the string values of an enum making this unusable for the configuration case - I'll switch that to a normal POJO with constants and getters/setters for a single value so I can use it in the internals to pass not string objects. Patched now

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

InstallationScope.UNSCOPED.name() ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

.name() is also not a constant expression (as it may be overridden) - getting corresponding errors.


private File homeDir;

private String scope;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use InstallationScope class

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

see comment above.. about to resolve.


import java.util.Arrays;

public class InstallationScope {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not enum?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tripodsan - see above, there is no way to access the values of the enum as a String with a static expression which I need for the annotations at build time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

well, I think that having the benefit of the enum throughout the code outweighs to use 3 strings in the config :-)

@asfgit asfgit closed this in 7d19909 Apr 3, 2019
asfgit pushed a commit that referenced this pull request Oct 7, 2019
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.

2 participants