DmitryKey / luke

This is mavenised Luke: Lucene Toolbox Project
Apache License 2.0
1.54k stars 352 forks source link

fix edit columns #118

Closed yossivainshtein closed 6 years ago

yossivainshtein commented 6 years ago

@DmitryKey I forgot one (crucial) file in the previous PR.

You should see the edit columns button now (next to the "explain" button")

screen shot 2018-09-27 at 17 06 01
mocobeta commented 6 years ago

@DmitryKey This pull request is pending in a while. How do we handle this?

As we announced when releasing version 7.3.1, the support for thinlet luke branch has been stopped. So originally speaking, we should not accept pull requests for this branch. I do not fully understand the context, but seems the review process has been started, and the first PR was already merged.

I do not have strong opinion about how to proceed this, but I think it should be completed by some form.

yossivainshtein commented 6 years ago

@DmitryKey @mocobeta Hey, I'm not sure what you decide to do, but as you said, the first PR was already merged but due to a bug the feature isn't shown.

So you should either accept this PR (with the fix) or revert back to before the fix. Personally I hope it'll be accepted, I know quite a lot of people working with earlier versions of lucene and hence Luke.

mocobeta commented 6 years ago

I'm not sure what you decide to do

When release 7.3.1, we announced as follows at the Luke Google Group and Lucene mailing list:

From this release, we will update JavaFX version only and stop maintenance Thinlet Luke. 

And, we added the notice in the repo's README:

From version 7.3.1, the main branch is running JavaFX (later, it was switched to Swing) version luke and Thinlet version is not maintained/updated. 

The reason of this decision is simple: We do not have sufficient development resources for maintaining old branches (We are volunteers and have full-time jobs, just like you.)

So you should either accept this PR (with the fix) or revert back to before the fix.

I do not think that this PR should be reverted back. I just mean, this should not be "pending" status. Anyway I am not sure about the circumstances. @DmitryKey will take care this, so please be patient.

mocobeta commented 6 years ago

Okay, I don't want to leave this hanging and of course you hope it'll be merged. Seems the original reviewer is too busy to take a look this, so I'll take this.

I will create two point releases - Thinlet Luke 7.2.0.1 and 4.10.4.1 (cherry-picked).

mocobeta commented 6 years ago

I created the releases:

Please take this is an exceptional case. We have stopped maintaining Thinlet Luke, to focus our limited resources on keeping the master branch up-to-date with the latest Lucene updates (So I won't announce the releases officially). Even though your commit looks nice to me.

Thanks @yossivainshtein!