Skip to content

Conversation

iwpnd
Copy link
Member

@iwpnd iwpnd commented Mar 11, 2025

description

  • drops GHA builtin services in favor of docker compose for more flexibility
  • also aligns local test environment with gha test environment

Would like to merge this before #1032 @ARolek, to separate it contextually. 😄

@iwpnd iwpnd requested review from ARolek and gdey as code owners March 11, 2025 17:52
@iwpnd iwpnd mentioned this pull request Mar 11, 2025
@coveralls
Copy link

coveralls commented Mar 11, 2025

Pull Request Test Coverage Report for Build 93ddf9b93-PR-1033

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 40.721%

Totals Coverage Status
Change from base Build 8b631ec81: 0.0%
Covered Lines: 6690
Relevant Lines: 16429

💛 - Coveralls

Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

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

Good catch!

@ARolek
Copy link
Member

ARolek commented Mar 11, 2025

govulncheck errors will be addressed in: #1034

@ARolek
Copy link
Member

ARolek commented Mar 12, 2025

@iwpnd can you please get rid of the "merge" commit on this branch. I don't think it's needed and we typically don't have those as part of the git history. This is ready to merge after that. I can also force push to your branch if you prefer.

@iwpnd
Copy link
Member Author

iwpnd commented Mar 12, 2025

Have I ever not rebased my PRs before merging @ARolek? 😏

@ARolek
Copy link
Member

ARolek commented Mar 12, 2025

@iwpnd I'm referring to dropping this commit: 8d40c39. I don't think its needed and it's a no op commit.

@iwpnd iwpnd force-pushed the ci/use-docker-compose branch from 8d40c39 to d9fe004 Compare March 12, 2025 16:34
@iwpnd
Copy link
Member Author

iwpnd commented Mar 12, 2025

Rebase will drop that is what I mean :)

@ARolek
Copy link
Member

ARolek commented Mar 12, 2025

Rebase will drop that is what I mean :)

I don't think it will, but let's give it a go. I have changed your permissions so you should be able to "rebase and merge" this PR into master.

@iwpnd
Copy link
Member Author

iwpnd commented Mar 12, 2025

Rebase will drop that is what I mean :)

I don't think it will, but let's give it a go. I have changed your permissions so you should be able to "rebase and merge" this PR into master.

Once we have the last review, yesss

@ARolek
Copy link
Member

ARolek commented Mar 12, 2025

Approved!

@iwpnd
Copy link
Member Author

iwpnd commented Mar 12, 2025

but @gdey is missing 😋

@gdey
Copy link
Member

gdey commented Mar 12, 2025

but @gdey is missing 😋

I'll look at it right now. Last time I check it was still a Draft. Thanks for the notification.

Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

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

I love when we can drop redundancy!

@iwpnd
Copy link
Member Author

iwpnd commented Mar 12, 2025

Hm, still not authorized to merge.

@gdey
Copy link
Member

gdey commented Mar 12, 2025

Hm, still not authorized to merge.

@iwpnd Can you try again. I added you as a core maintainer.

@iwpnd iwpnd merged commit c608ce7 into go-spatial:master Mar 13, 2025
12 checks passed
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.

4 participants