Skip to content

SOLR-15602: upgrade to gradle 7.2, clean up gradle code a bit. #275

Merged
dweiss merged 9 commits intoapache:mainfrom
dweiss:SOLR-15602
Sep 1, 2021
Merged

SOLR-15602: upgrade to gradle 7.2, clean up gradle code a bit. #275
dweiss merged 9 commits intoapache:mainfrom
dweiss:SOLR-15602

Conversation

@dweiss
Copy link
Copy Markdown
Contributor

@dweiss dweiss commented Aug 28, 2021

No description provided.

build.gradle Outdated
Comment on lines 23 to 28
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: end of line whitespace

build.gradle Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we call this "placeholder-outputs" instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep it consistent with Lucene, if you don't mind. Not a native speaker but is something wrong with the "dummy"? I am aware of other senses but this literal definition suits me just fine in this context "an object designed to resemble and serve as a substitute for the real or usual one."?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be RegularFile?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't have to be. It can be plain Java API. We're not being fancy with the lazy API here so I don't think we need to make things complicated? There is a semantic difference between Input and InputFile - this was a bug.

https://docs.gradle.org/current/userguide/more_about_tasks.html#sec:up_to_date_checks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need some kind of flag to regenerate this file if it exists but users are upgrading from an older gradle/jvm version?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered about this too, but in the end opted for simplicity. This initial generated settings file is meant to provide the defaults and pointers about where other things can be tweaked. But it is meant to be tweaked to your (the developer's) needs and I know some folks are doing just that. Manipulating their changes would only create more headaches. You can always wipe this file and let it regenerate if you want it refreshed entirely.

I think these days you could also use init scripts to set up your local Solr builds completely independently from the repository structure [1]. I haven't toyed with this though.

https://docs.gradle.org/current/userguide/init_scripts.html#sec:using_an_init_script

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markrmiller was updating this in #225 just in case that gets merged first

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this syntax and am having trouble searching for it. Can you help me find a reference?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're familiar with it, you just don't know it. :) I recall I head-scratched over the very same thing... It's a gstring (lazily evaluated string) with a snippet of gradle code which happens to be a parameterless closure. The jar plugin is actually better now and supports lazy providers... So this could be modified entirely. I just copied the solution that worked in Lucene.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dweiss dweiss merged commit 6131709 into apache:main Sep 1, 2021
epugh pushed a commit to epugh/solr that referenced this pull request Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants