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-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org