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

Add release info for publishing to Maven Central #246

Merged
merged 2 commits into from
Jul 25, 2023
Merged

Conversation

eloots
Copy link
Collaborator

@eloots eloots commented Jul 25, 2023

No description provided.

@eloots eloots requested a review from ckipp01 July 25, 2023 10:46
@ckipp01
Copy link
Member

ckipp01 commented Jul 25, 2023

Alright, so you'll need to rebase after I merged in the original one. Also, one more thing. The create-release job is currently dependant on a tag push, so it won't actually run the release. I'm not sure how you want this set up, but it might be that you change the logic there around. That's typically why you see a separate release.yml is that it allows you to more easily control the triggers. Either way, once these changes get added everything should be good to go.

@eloots
Copy link
Collaborator Author

eloots commented Jul 25, 2023

@ckipp01 Rebase done.

build.sbt Outdated
@@ -9,6 +11,12 @@ lazy val `course-management-tools` =
.aggregate(cmta, cmtc, core, `functional-tests`, docs)
.settings(commonSettings: _*)
.settings(publish / skip := true)
.settings(inThisBuild(List(
Copy link
Member

Choose a reason for hiding this comment

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

Sort of a total nit, but if you're using inThisBuild it doesn't actually need to be nested in here. It can live at the top level and also work fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense; redundant stuff shouldn't be present. Removing it.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, the change you made isn't the same. So you'll notice now that your settings are only applying to the aggregate, not all projects. So for example if you would look at the developers set for cmta you'll see there is none. So you'd need to either use inThisBuild at the top level, or move the settings themselves into your commonSettings which then are given to each module.

@eloots eloots merged commit b4706c7 into main Jul 25, 2023
3 checks passed
@eloots eloots deleted the add-release-info branch July 25, 2023 11:27
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