You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by GitBox <gi...@apache.org> on 2022/06/12 02:05:39 UTC
[GitHub] [directory-studio] roubert opened a new pull request, #37: DIRSTUDIO-1300: Use LookupTranslator instead of chained replace() calls.
roubert opened a new pull request, #37:
URL: https://github.com/apache/directory-studio/pull/37
https://issues.apache.org/jira/browse/DIRSTUDIO-1300
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-studio] seelmann commented on a diff in pull request #37: DIRSTUDIO-1300: Use LookupTranslator instead of chained replace() calls.
Posted by GitBox <gi...@apache.org>.
seelmann commented on code in PR #37:
URL: https://github.com/apache/directory-studio/pull/37#discussion_r895208407
##########
pom.xml:
##########
@@ -137,8 +139,8 @@
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-compiler-plugin</artifactId>
<configuration>
- <source>1.8</source>
- <target>1.8</target>
+ <source>11</source>
+ <target>11</target>
Review Comment:
Thanks!
##########
plugins/ldapbrowser.core/src/main/java/org/apache/directory/studio/ldapbrowser/core/jobs/ExportOdfRunnable.java:
##########
@@ -60,6 +61,9 @@ public class ExportOdfRunnable implements StudioConnectionRunnableWithProgress
/** The maximum count limit */
public static final int MAX_COUNT_LIMIT = 65000;
+ /** The postal address decoder. */
+ private static CharSequenceTranslator DECODER = Utils.createPostalAddressDecoder( "\n" ); //$NON-NLS-1$;
Review Comment:
According to the javadoc all translators are thread-safe so the singleton is fine.
##########
plugins/valueeditors/pom-first.xml:
##########
@@ -64,10 +64,12 @@
org.apache.directory.studio.valueeditors.uuid</Export-Package>
<Import-Package>org.apache.commons.codec,
- org.apache.commons.codec.binary
+ org.apache.commons.codec.binary,
+ org.apache.commons.text
</Import-Package>
<Require-Bundle>
+ org.apache.commons.commons-text,
Review Comment:
Is the bundle required if the package is imported, or vice versa?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-studio] roubert commented on a diff in pull request #37: DIRSTUDIO-1300: Use LookupTranslator instead of chained replace() calls.
Posted by GitBox <gi...@apache.org>.
roubert commented on code in PR #37:
URL: https://github.com/apache/directory-studio/pull/37#discussion_r895209532
##########
plugins/valueeditors/pom-first.xml:
##########
@@ -64,10 +64,12 @@
org.apache.directory.studio.valueeditors.uuid</Export-Package>
<Import-Package>org.apache.commons.codec,
- org.apache.commons.codec.binary
+ org.apache.commons.codec.binary,
+ org.apache.commons.text
</Import-Package>
<Require-Bundle>
+ org.apache.commons.commons-text,
Review Comment:
I don't really understand how these dependency declarations work so what's here is all the result of trial-and-error (but the result is that both `build.sh` and Eclipse can build and run this the way it is now). If you or someone else actually knows how this really works, you might want to take a look afterwards and see if you can find things to correct or simplify.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-studio] seelmann merged pull request #37: DIRSTUDIO-1300: Use LookupTranslator instead of chained replace() calls.
Posted by GitBox <gi...@apache.org>.
seelmann merged PR #37:
URL: https://github.com/apache/directory-studio/pull/37
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-studio] seelmann commented on a diff in pull request #37: DIRSTUDIO-1300: Use LookupTranslator instead of chained replace() calls.
Posted by GitBox <gi...@apache.org>.
seelmann commented on code in PR #37:
URL: https://github.com/apache/directory-studio/pull/37#discussion_r895212780
##########
plugins/valueeditors/pom-first.xml:
##########
@@ -64,10 +64,12 @@
org.apache.directory.studio.valueeditors.uuid</Export-Package>
<Import-Package>org.apache.commons.codec,
- org.apache.commons.codec.binary
+ org.apache.commons.codec.binary,
+ org.apache.commons.text
</Import-Package>
<Require-Bundle>
+ org.apache.commons.commons-text,
Review Comment:
`Require-Bundle` means to use that specific bundle/dependency, and all packages in that bundle are imported.
`Import-Package` means to only import the declared packages, so it's more fine-grained. The actual implementation of the packagesis open at the `MANIFEST.MF` level. The pb is also that those declarations are not transitive, so I assume we need to import `org.apache.commons.text` and `org.apache.commons.text.translate`.
Fine to keep it like this, I'll have a look later if we can simplify.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org