You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "s1monw (via GitHub)" <gi...@apache.org> on 2024/02/12 14:37:33 UTC
[PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
s1monw opened a new pull request, #13095:
URL: https://github.com/apache/lucene/pull/13095
In #13046 several changes broke the addBackcompatIndexes.py script to properly add and test the unreleased version. This updates the script to again properly add the new version.
Closes #13094
### Description
<!--
If this is your first contribution to Lucene, please make sure you have reviewed the contribution guide.
https://github.com/apache/lucene/blob/main/CONTRIBUTING.md
-->
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1938959760
Great! I haven't seen we use `//spotless:off` already and its enabled in the config. So I think this is best option here. This also allows the regexes for the script to not break suddenly.
I don't know why the PR's precommit checks fail. It complains about missing classes....
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "s1monw (via GitHub)" <gi...@apache.org>.
s1monw commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1944584231
@uschindler dude I need to press buttons it took me 14 mins to port I guess that's ok?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "s1monw (via GitHub)" <gi...@apache.org>.
s1monw merged PR #13095:
URL: https://github.com/apache/lucene/pull/13095
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1938862855
What's the exact problem with tidy?
I think after adding the new version, we should run tidy to let it cleanup the code.
I will check if there are some comment signals to tidy so it excludes some code from reformatting. Personally I'd like to do this sometimes because reformatting of code breaks readability.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1939072225
@uschindler I have been blindly following the release wizard, I'll look into the failure in a bit and see how we can address it.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486656215
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
...then it's good that you have not touched that test file :-) Of course if you accidentally do it you see it before committing (at least I check the code for unwanted formatting changes, because not everything is cleaned up by the formatter like where newlines are and so on.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486535126
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
thinking about it, having these versions in separate resources could also make it easier to manipulate from scripts?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1487803547
##########
dev-tools/scripts/addBackcompatIndexes.py:
##########
@@ -114,53 +113,25 @@ def find_version(x):
x = re.sub(strip_dash_suffix_re, '', x) # remove the -suffix if any
return scriptutil.Version.parse(x)
- class Edit(object):
- start = None
- def __call__(self, buffer, match, line):
- if self.start:
- # find where this version should exist
- i = len(buffer) - 1
- previous_version_exists = not ('};' in line and buffer[-1].strip().endswith("{"))
- if previous_version_exists: # Only look if there is a version here
- v = find_version(buffer[i])
- while i >= self.start and v.on_or_after(index_version):
- i -= 1
- v = find_version(buffer[i])
- i += 1 # readjust since we skipped past by 1
-
- # unfortunately python doesn't have a range remove from list...
- # here we want to remove any previous references to the version we are adding
- while i < len(buffer) and index_version.on_or_after(find_version(buffer[i])):
- buffer.pop(i)
-
- if i == len(buffer) and previous_version_exists and not buffer[-1].strip().endswith(","):
- # add comma
- buffer[-1] = buffer[-1].rstrip() + ",\n"
-
- if previous_version_exists:
- last = buffer[-1]
- spaces = ' ' * (len(last) - len(last.lstrip()))
- else:
- spaces = ' '
- for (j, t) in enumerate(types):
- if t == 'sorted':
- newline = spaces + ('"sorted.%s"') % index_version
- else:
- newline = spaces + ('"%s-%s"' % (index_version, t))
- if j < len(types) - 1 or i < len(buffer):
- newline += ','
- buffer.insert(i, newline + '\n')
- i += 1
-
- buffer.append(line)
- return True
-
- if 'Names = {' in line:
- self.start = len(buffer) # location of first index name
- buffer.append(line)
- return False
+ def edit(buffer, match, line):
Review Comment:
Oh that's so much simpler, great! +1
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestAncientIndicesCompatibility.java:
##########
@@ -36,222 +41,36 @@
import org.apache.lucene.tests.store.BaseDirectoryWrapper;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.apache.lucene.tests.util.TestUtil;
+import org.apache.lucene.util.IOUtils;
-@SuppressWarnings("deprecation")
public class TestAncientIndicesCompatibility extends LuceneTestCase {
+ static final Set<String> UNSUPPORTED_VERSIONS;
Review Comment:
Actually, this is a list of filenames and not versions.
I would rename the Set to `UNSUPPORTED_INDEXES` and the same for the `txt` file.
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -48,25 +52,28 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ static final Set<String> OLD_VERSIONS;
+ protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
+
private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion();
+
+ protected final Version version;
protected final String indexPattern;
- protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
static {
- String[] oldVersions =
- new String[] {
- "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0",
- "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1",
- "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3",
- "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0",
- "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2",
- "8.11.2", "8.11.3", "8.11.3", "8.12.0", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0",
- "9.4.1", "9.4.2", "9.5.0", "9.6.0", "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2"
- };
-
+ String name = "versions.txt";
+ try (LineNumberReader in =
+ new LineNumberReader(
+ IOUtils.getDecodingReader(
+ IOUtils.requireResourceNonNull(
+ TestAncientIndicesCompatibility.class.getResourceAsStream(name), name),
+ StandardCharsets.UTF_8))) {
+ OLD_VERSIONS = in.lines().collect(Collectors.toCollection(LinkedHashSet::new));
Review Comment:
💖
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -48,25 +52,28 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ static final Set<String> OLD_VERSIONS;
+ protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
+
private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion();
+
+ protected final Version version;
protected final String indexPattern;
- protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
static {
- String[] oldVersions =
- new String[] {
- "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0",
- "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1",
- "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3",
- "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0",
- "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2",
- "8.11.2", "8.11.3", "8.11.3", "8.12.0", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0",
- "9.4.1", "9.4.2", "9.5.0", "9.6.0", "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2"
- };
-
+ String name = "versions.txt";
Review Comment:
This code looks good to me. At beginning I wondered why we used properties at all, because those are no key/value pairs. I later figured out that only the `keySet()` was used and transformed to an array.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486640670
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
Anyways, I agree to have those 2 lists to be a resource file and loaded using `Class#getResourceAsStream()` and split by lines.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1938981480
> @uschindler i think it's because @jpountz added a new version on branch_9x I spoke to him already I think it breaks tests as well on the stable branch
Yeah, I also see this one on Jenkins:
```
> Task :lucene:core:compileJava
C:\Users\jenkins\workspace\Lucene-MMAPv2-Windows\lucene\core\src\java\org\apache\lucene\util\Version.java:139: warning: [dep-ann] deprecated item is not annotated with @Deprecated
@Deprecated public static final Version LUCENE_9_10_0 = new Version(9, 10, 0);
^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
1 error
1 warning
```
It has a duplicate annotation:
https://github.com/apache/lucene/blob/314c553bdce22bc1bc08e46b218691575c2287e0/lucene/core/src/java/org/apache/lucene/util/Version.java#L133-L139
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486656215
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
...then it's good that you have not touched that test file :-)
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1488362426
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -48,25 +53,31 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ static final Set<String> OLD_VERSIONS;
+ protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
+
private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion();
+
+ protected final Version version;
protected final String indexPattern;
- protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
static {
- String[] oldVersions =
- new String[] {
- "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0",
- "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1",
- "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3",
- "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0",
- "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2",
- "8.11.2", "8.11.3", "8.11.3", "8.12.0", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0",
- "9.4.1", "9.4.2", "9.5.0", "9.6.0", "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2"
- };
-
+ String name = "versions.txt";
+ try (LineNumberReader in =
+ new LineNumberReader(
+ IOUtils.getDecodingReader(
+ IOUtils.requireResourceNonNull(
+ TestAncientIndicesCompatibility.class.getResourceAsStream(name), name),
Review Comment:
Small nit: Copy-paste error, wrong class name.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1941375395
I've broken it, I'll fix it in a sec.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1944578996
This was merged to 9.x only. Should we not have it in main, too?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "s1monw (via GitHub)" <gi...@apache.org>.
s1monw commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1944559092
> P.S.: I don't know if we still need those 4 special case indexes. They seem to be a relic from earlier times. As we only check that the indexes throw an Exception on loading, the already existing 5.x and 4.x indexes are enough.
true, I will merge this one and open a new pr to remove these
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "s1monw (via GitHub)" <gi...@apache.org>.
s1monw commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1938895854
@uschindler this works. it's funky if one of the strings has a certain length it will allow the bock-like strucuture.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1487821423
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -48,25 +52,28 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ static final Set<String> OLD_VERSIONS;
+ protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
+
private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion();
+
+ protected final Version version;
protected final String indexPattern;
- protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
static {
- String[] oldVersions =
- new String[] {
- "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0",
- "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1",
- "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3",
- "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0",
- "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2",
- "8.11.2", "8.11.3", "8.11.3", "8.12.0", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0",
- "9.4.1", "9.4.2", "9.5.0", "9.6.0", "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2"
- };
-
+ String name = "versions.txt";
+ try (LineNumberReader in =
+ new LineNumberReader(
+ IOUtils.getDecodingReader(
+ IOUtils.requireResourceNonNull(
+ TestAncientIndicesCompatibility.class.getResourceAsStream(name), name),
+ StandardCharsets.UTF_8))) {
+ OLD_VERSIONS = in.lines().collect(Collectors.toCollection(LinkedHashSet::new));
Review Comment:
I think this is mainly a problem with trailing newlines. The documentation of `BufferedReader#lines()` does not mention any cleanup.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1939064842
@jpountz fixed the duplicate `@Deprecated` but it still fails because it can't find v9.11.0. I have no idea how it handles the last version of a previous stable branch. For sure from main's point of view 9.11.0 was never released.
At least for this PR I think it looks fine. The spotless trick solves the issue. +1
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1488367811
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -48,25 +53,31 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ static final Set<String> OLD_VERSIONS;
+ protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
+
private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion();
+
+ protected final Version version;
protected final String indexPattern;
- protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
static {
- String[] oldVersions =
- new String[] {
- "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0",
- "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1",
- "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3",
- "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0",
- "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2",
- "8.11.2", "8.11.3", "8.11.3", "8.12.0", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0",
- "9.4.1", "9.4.2", "9.5.0", "9.6.0", "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2"
- };
-
+ String name = "versions.txt";
+ try (LineNumberReader in =
+ new LineNumberReader(
+ IOUtils.getDecodingReader(
+ IOUtils.requireResourceNonNull(
+ TestAncientIndicesCompatibility.class.getResourceAsStream(name), name),
Review Comment:
Darn.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486637570
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
P.S.: In this text above the formatting seems to have a reason because the upper and lower line somehow aligned to each other (the lower line has 1000 added to all entries and the first one is missing). So it looks like this demonstrates some gap in the positions: https://github.com/apache/lucene/blob/027240f522e32001d2daf6a622aeb7fbe87f8869/lucene/queries/src/test/org/apache/lucene/queries/payloads/TestPayloadCheckQuery.java#L270-L287
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486881008
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
> Should we move these constants to a resource file as part of this issue or a separate one?
I think this should be part of this PR. Especially as it requires changes in the scripts, too.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1487826242
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -50,7 +52,7 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- static final String[] OLD_VERSIONS;
+ static final Set<String> OLD_VERSIONS;
Review Comment:
I think the String sets can be private, as we only want to expose the Version numbers, correct?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486531645
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
I don't think this is such a great idea... in general, google java format does not have any way to prevent formatting - it's all or nothing (this is a deliberate decision).
I think a better way to fix this would be to:
* read this from a resource, which can be a plain text file formatted in any way you like,
* use // comments on each line to force line breaks for google java formatter (ugly but will work).
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486550352
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
In case I'm not clear - GJF (google java formatter) is "used by" spotless but you can also use it independently. As for exclusion markup - here is the faq entry from GJF concerning on/off markup (there are also discussions on their issue tracker).
https://github.com/google/google-java-format/wiki/FAQ#why-cant-i-use-magic-comments-to-disable-formatting-for-certain-sections-of-code
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486645486
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
> I understand you comment about IntelliJ, but why should I appliy the Google Java format there when gradle can do it?
Accidently. I format the code in intellij all the time - I may not see the formatter-exclusion tags and intellij would reformat that part - a subsequent gradle tidy run wouldn't touch this section but it would remain reformatted.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486656215
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
...then it's good that you have not touched that test file :-) Of course if you accidentally do it, you see it before committing (at least I check the code for unwanted formatting changes, because not everything is cleaned up by the formatter like where newlines are and so on).
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486879238
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
> I feel inclined to remove those existing two sections and add it to the rat pre-check, actually.
It's enough to disable the setting in the gradle build: https://github.com/apache/lucene/blob/027240f522e32001d2daf6a622aeb7fbe87f8869/gradle/validation/spotless.gradle#L31
It was enabled in [LUCENE-9659](https://issues.apache.org/jira/browse/LUCENE-9659)
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1938869700
I'd like for the reasons described before to enable this option:
https://stackoverflow.com/a/76221803
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486879238
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
> I feel inclined to remove those existing two sections and add it to the rat pre-check, actually.
It's enough to disable the setting in the gradle build: https://github.com/apache/lucene/blob/027240f522e32001d2daf6a622aeb7fbe87f8869/gradle/validation/spotless.gradle#L31
It was enabled in https://github.com/apache/lucene-solr/pull/2185
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486879238
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
> I feel inclined to remove those existing two sections and add it to the rat pre-check, actually.
It's enough to disable the setting in the gradle build: https://github.com/apache/lucene/blob/027240f522e32001d2daf6a622aeb7fbe87f8869/gradle/validation/spotless.gradle#L31
It was enabled in #2185
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1941331344
I pushed a simple patch replacing properties with txt files - no need to worry about property file syntax and easier to read too, I think.
I like how python script got simpler too.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1488345592
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -48,25 +52,28 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ static final Set<String> OLD_VERSIONS;
+ protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
+
private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion();
+
+ protected final Version version;
protected final String indexPattern;
- protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
static {
- String[] oldVersions =
- new String[] {
- "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0",
- "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1",
- "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3",
- "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0",
- "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2",
- "8.11.2", "8.11.3", "8.11.3", "8.12.0", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0",
- "9.4.1", "9.4.2", "9.5.0", "9.6.0", "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2"
- };
-
+ String name = "versions.txt";
+ try (LineNumberReader in =
+ new LineNumberReader(
+ IOUtils.getDecodingReader(
+ IOUtils.requireResourceNonNull(
+ TestAncientIndicesCompatibility.class.getResourceAsStream(name), name),
+ StandardCharsets.UTF_8))) {
+ OLD_VERSIONS = in.lines().collect(Collectors.toCollection(LinkedHashSet::new));
Review Comment:
I see Simon already applied that (thank you!).
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1944599169
Sorry, I was just irritated, because the original PR was opened against stable branch. 😜
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1488370861
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -48,25 +53,31 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ static final Set<String> OLD_VERSIONS;
+ protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
+
private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion();
+
+ protected final Version version;
protected final String indexPattern;
- protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
static {
- String[] oldVersions =
- new String[] {
- "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0",
- "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1",
- "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3",
- "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0",
- "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2",
- "8.11.2", "8.11.3", "8.11.3", "8.12.0", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0",
- "9.4.1", "9.4.2", "9.5.0", "9.6.0", "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2"
- };
-
+ String name = "versions.txt";
+ try (LineNumberReader in =
+ new LineNumberReader(
+ IOUtils.getDecodingReader(
+ IOUtils.requireResourceNonNull(
+ TestAncientIndicesCompatibility.class.getResourceAsStream(name), name),
Review Comment:
Fixed.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "s1monw (via GitHub)" <gi...@apache.org>.
s1monw commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1941123474
@uschindler @dweiss I moved all version to property files and updated the python script
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "s1monw (via GitHub)" <gi...@apache.org>.
s1monw commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1938967335
@uschindler i think it's because @jpountz added a new version on branch_9x I spoke to him already I think it breaks tests as well on the stable branch
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486628856
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
It looks like `spotless:off` works fine.
I tried it out in https://github.com/apache/lucene/blob/main/lucene/queries/src/test/org/apache/lucene/queries/payloads/TestPayloadCheckQuery.java and it worked as expected:
- I changed indent and code outside the block: formatting was restored
- I changed indent in the code of those array sections: formatting was not updated by formatter
So it looks like GJF does not support the tag, but spotless does.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486539904
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
I agree, maybe 2 resource files would be better.
To your first comment: there is `spotless:off` and `spotless:on` and we use it already in another source file. Why is it all or nothing although it is documented in a different way? It this because of a Google Java Format special case?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "s1monw (via GitHub)" <gi...@apache.org>.
s1monw commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1941931700
@uschindler I did separate out the specific indices and added a new file. that makes maintenance simpler and moves the `-cfs` `-nocfs` duplication to the code. Also applied you comments.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "s1monw (via GitHub)" <gi...@apache.org>.
s1monw commented on PR #13095:
URL: https://github.com/apache/lucene/pull/13095#issuecomment-1938803239
this fixes the release script but I can't get tidy to not wrap the array [here](https://github.com/apache/lucene/pull/13095/files#diff-8946e557343c70aadb734c2f1bdceeed366f32c6f667ea75b89b89ae2895887fR50) @uschindler or @rjernst can you help me to find out what's going on....
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486545976
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
When you use google java formatter from intellij, for example, it will ignore spotless markup and reformat the source anyway.
Any such remaining markup is a mistake. It will keep piling up - it's almost never a good idea to have it.
I think those versions should be kept in plain text resources - then updating them will be easier as well?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486632751
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
I understand you comment about IntelliJ, but why should I appliy the Google Java format there when gradle can do it?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1486846219
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
I feel inclined to remove those existing two sections and add it to the rat pre-check, actually. I can think of better ways of conveying the meaning of those "aligned" source lines.
Should we move these constants to a resource file as part of this issue or a separate one?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1487332805
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -47,25 +47,64 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ // spotless:off
Review Comment:
Thanks, Uwe. I'll revert that option. It's a problem waiting to happen. I may help straightening this issue too but in the evening - on the road today.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix addBackcompatIndexes.py to properly generate missing versions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13095:
URL: https://github.com/apache/lucene/pull/13095#discussion_r1487811954
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##########
@@ -48,25 +52,28 @@
public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase {
- protected final Version version;
+ static final Set<String> OLD_VERSIONS;
+ protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
+
private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion();
+
+ protected final Version version;
protected final String indexPattern;
- protected static final Set<Version> BINARY_SUPPORTED_VERSIONS;
static {
- String[] oldVersions =
- new String[] {
- "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0",
- "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1",
- "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3",
- "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0",
- "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2",
- "8.11.2", "8.11.3", "8.11.3", "8.12.0", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0",
- "9.4.1", "9.4.2", "9.5.0", "9.6.0", "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2"
- };
-
+ String name = "versions.txt";
+ try (LineNumberReader in =
+ new LineNumberReader(
+ IOUtils.getDecodingReader(
+ IOUtils.requireResourceNonNull(
+ TestAncientIndicesCompatibility.class.getResourceAsStream(name), name),
+ StandardCharsets.UTF_8))) {
+ OLD_VERSIONS = in.lines().collect(Collectors.toCollection(LinkedHashSet::new));
Review Comment:
One comment: Should we not have a `filter(Predicate.not(String::isBlank)`?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org