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

First Steps into making Gradle Catalogs #3724

Merged
merged 6 commits into from
Jul 29, 2023
Merged

First Steps into making Gradle Catalogs #3724

merged 6 commits into from
Jul 29, 2023

Conversation

JBassett
Copy link
Collaborator

Summary

Gradle Catalogs!

Screenshots

N/A

Link to pull request in Documentation repository

N/A

Any other notes

@JBassett JBassett marked this pull request as ready for review July 28, 2023 18:09
@JBassett
Copy link
Collaborator Author

I realize these might not be named exactly how we want, if anyone sees anything they want to see changed please add a review and I'll refactor it.

@jpelgrom
Copy link
Member

Two things that come to mind, but maybe you intend do it in a later stage:

@JBassett
Copy link
Collaborator Author

Two things that come to mind, but maybe you intend do it in a later stage:

So I tried to keep it to as much of a 1 to 1 mapping of existing to new, due to differences in how plugins are declared currently it wasn't as easy and I wanted to do separately. As far as the bundling I think handling them separately will make the PRs cleaner and easier to read.

@jpelgrom
Copy link
Member

So I tried to keep it to as much of a 1 to 1 mapping of existing to new, due to differences in how plugins are declared currently it wasn't as easy and I wanted to do separately. As far as the bundling I think handling them separately will make the PRs cleaner and easier to read.

Sounds like you've thought about it, good plan.

Copy link
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

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

Names that I noticed while scanning the list / weren't obvious to me until I looked at the toml file

gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
Copy link
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

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

One more name suggestion. I also noticed some dependencies that weren't sorted correctly when doing this a-z but feel free to disregard if you don't agree that would be a good way to sort these.

gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
@JBassett
Copy link
Collaborator Author

One more name suggestion. I also noticed some dependencies that weren't sorted correctly when doing this a-z but feel free to disregard if you don't agree that would be a good way to sort these.

Fixed the comments. For now alphabetized will work, as I start grouping them we might sort in order of perceived importance, or by which module they are used.

Copy link
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

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

Let's merge this so more improvements can happen, we can always change more names later

@JBassett JBassett merged commit ba31703 into master Jul 29, 2023
@JBassett JBassett deleted the feature/cataloging branch July 29, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants