Skip to content
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

Support putting run configs in folders #1276

Open
wants to merge 4 commits into
base: dev/1.10
Choose a base branch
from

Conversation

SquidDev
Copy link
Contributor

  • Remove unused RunConfig.genRuns.
  • Rewrite templating to do a single find-and-replace, rather than a chain of them.
  • Add a new ideConfigFolder option, which sets the folderName attribute on the IDEA config file.

IntellJ Idea's run config drop down. There is a folder called "Fabric", where which contains a list of runs (e.g. client, server).

I wasn't quite sure the best way to expose settings from RunConfigSettings, as it's currently a bit ad-hoc. For now, I've gone with the simplest option (a Property<String> getter), but happy to change that if preferred!

Given how many of my projects use checkstyle, you'd have thought I'd
remember to check it before submitting a PR.
"I won't run the tests, I've tested manually, and nothing will break".
Words spoken before disaster (or at least making a fool of myself in
CI).
@SquidDev
Copy link
Contributor Author

SquidDev commented Mar 14, 2025

I did have a think about replacing IdeaClasspathModificationsTest.fromDummy with a call to RunConfig.fromDummy. Technically we can get away passing a null Project to it, but it felt a bit risky, so have left as-is for now.

@modmuss50
Copy link
Member

I did have a think about replacing IdeaClasspathModificationsTest.fromDummy with a call to RunConfig.fromDummy. Technically we can get away passing a null Project to it, but it felt a bit risky, so have left as-is for now.

I'd leave it for now, the test coverage (and testability) of all of this could be improved vastly, but there is no point doing it as part of this PR. Ill take a closer look at the PR shortly and get it merged into the 1.11 branch.

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