New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

better? #70

Merged
merged 2 commits into from Jan 29, 2013

Conversation

Projects
None yet
2 participants
@srse
Contributor

srse commented Jan 23, 2013

seleniumFixV2: connected thread pool creation to condition web.allowLuceneIndexing (otherwise thread pool will be kept open for more than a minute, what hinders the start of another gitblit instance in the same jvm during selenium test case execution), fixed naming of xpath variables, added missing property to gitblit.properties and test-ui-gitblit.properties, changed naming of methods (according to decision that owners is a much shorter word than repository administrator and that owners is fits better if you think of collective responsibilities and collective ownership)

SHaselbauer
seleniumFixV2: connected thread pool creation to condition web.allowL…
…uceneIndexing (otherwise thread pool will be kept open for more than a minute, what hinders the start of another gitblit instance in the same jvm during selenium test case execution), fixed naming of xpath variables, added missing property to gitblit.properties and test-ui-gitblit.properties, changed naming of methods (according to decision that owners is a much shorter word than repository administrator and that owners is fits better if you think of collective responsibilities and collective ownership)
boolean luceneIndexing = settings.getBoolean(Keys.web.allowLuceneIndexing, true);
logger.info("Lucene indexing is " + (luceneIndexing ? "" : "not") + " activated");
if (luceneIndexing) {

This comment has been minimized.

@gitblit

gitblit Jan 23, 2013

Owner

It's better, although it is currently redundant.
https://github.com/gitblit/gitblit/blob/master/src/com/gitblit/LuceneExecutor.java#L161

If you think this is an improvement (I'm indifferent) then we should remove the check in the executor and also update gitblit.properties to indicate that this setting will now require a server restart.
https://github.com/gitblit/gitblit/blob/master/distrib/gitblit.properties#L572

Like, for example, https://github.com/gitblit/gitblit/blob/master/distrib/gitblit.properties#L704

This comment has been minimized.

@srse

srse Jan 23, 2013

Contributor

ScheduledThreadPoolExecutor.scheduleAtFixedRate calls delayedExecute thus there are still Threads running after gitblit has been terminated, but while the JVM is still running. So, for selenium test suites it will cause a test case fail for all test cases, but the first one. Anyway, I would prefer to keep the setting changeable on runtime (without a restart). I have another idea how to deal with this. See the next commit.

gitblit added a commit that referenced this pull request Jan 29, 2013

@gitblit gitblit merged commit 58102e9 into gitblit:master Jan 29, 2013

gitblit added a commit that referenced this pull request May 12, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment