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

chore: Makefile cleanup #1995

Merged
merged 31 commits into from
Aug 10, 2023
Merged

chore: Makefile cleanup #1995

merged 31 commits into from
Aug 10, 2023

Conversation

sfc-gh-asawicki
Copy link
Collaborator

@sfc-gh-asawicki sfc-gh-asawicki commented Aug 4, 2023

Changes

Overall Makefile cleanup:

  • removing old recipes
  • updating recipes
  • fixing recipes
  • removing dead code
  • introducing useful macros
  • grouping recipes
  • adding dependencies
  • updating help descriptions

There are still some open discussions, so more changes will be added here or to follow-up PRs.

Test Plan

  • CI run tests
  • manual tests by DEVs

@github-actions
Copy link

github-actions bot commented Aug 4, 2023

Integration tests success for 37fb79ad1b5c280ebce49c4d148fe7f719b0b1da

Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Aug 7, 2023

Integration tests success for 0db1aa0bcba7d9da427db64d5765ea3a06ce3c5a

@github-actions
Copy link

github-actions bot commented Aug 7, 2023

Integration tests success for 7273ce42aab16706343a484420eb610060de22fd

@github-actions
Copy link

github-actions bot commented Aug 7, 2023

Integration tests success for 2aa31f6aa3f3a9d422b98b936438aba77686d238

@sfc-gh-asawicki sfc-gh-asawicki marked this pull request as ready for review August 9, 2023 09:31
Makefile Outdated
CGO_ENABLED=1 $(go_test) -race -coverprofile=coverage.txt -covermode=atomic $(TESTARGS) ./pkg/snowflake/...

test: ## run the tests (except sdk tests)
CGO_ENABLED=1 go test -race $(COVERAGE_FLAGS) ./pkg/resources/...
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we not running data source tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these are only unit tests which are now run by Unit tests giuthub action (defined in file unit.yml). TBH I would like to remove it also, and remove it from unit.yml, but I was not sure if someone is using it independently (I wrote my concerns in the doc that I have shared internally).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can remove it also, why not :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The name "unit test" is confusing, because its not including any unit tests from SDK. There may be some minimal "unit tests" that test specific helper functions / validation functions for the provider,

perhaps we should create two different workflows to test the SDK and the Terraform Provider. That would make it easier to split later. And then these makefile commands could be more specific such as make unit-test provider or something to that effect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed.

Makefile Outdated
CGO_ENABLED=1 $(go_test) -race -coverprofile=coverage.txt -covermode=atomic $(TESTARGS) ./pkg/snowflake/...

test: ## run the tests (except sdk tests)
CGO_ENABLED=1 go test -race $(COVERAGE_FLAGS) ./pkg/resources/...
Copy link
Collaborator

Choose a reason for hiding this comment

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

The name "unit test" is confusing, because its not including any unit tests from SDK. There may be some minimal "unit tests" that test specific helper functions / validation functions for the provider,

perhaps we should create two different workflows to test the SDK and the Terraform Provider. That would make it easier to split later. And then these makefile commands could be more specific such as make unit-test provider or something to that effect.

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

Integration tests failure for 1aadca12d81a5e4ee202dbb962c0cdfaf56d4723

@github-actions
Copy link

github-actions bot commented Aug 9, 2023

Integration tests success for 5ec612e83ca08fa38301a5336c270b1e2b6a46ba

sfc-gh-pbosak
sfc-gh-pbosak previously approved these changes Aug 10, 2023
Makefile Outdated Show resolved Hide resolved
.github/workflows/unit.yml Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@github-actions
Copy link

Integration tests success for d8a718f15cc3fe9a247bc9ee466c9f5960db1224

@github-actions
Copy link

Integration tests failure for 835d683fd22dac0ed1481e2764278fa70e92b466

@github-actions
Copy link

Integration tests success for 835d683fd22dac0ed1481e2764278fa70e92b466

@github-actions
Copy link

Integration tests failure for 96fd3c33caeebbfbfd95dc03eb5150cc100f84de

@github-actions
Copy link

Integration tests success for 96fd3c33caeebbfbfd95dc03eb5150cc100f84de

@sfc-gh-asawicki sfc-gh-asawicki merged commit 5c6fdbe into main Aug 10, 2023
8 checks passed
@sfc-gh-asawicki sfc-gh-asawicki deleted the makefile-cleanup branch August 10, 2023 17:02
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.

None yet

5 participants