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

Minor code updates from missed open comments from #36 to the migratedown feature #37

Merged
merged 4 commits into from
Oct 9, 2024

Conversation

venkat-iblox
Copy link

@venkat-iblox venkat-iblox commented Oct 4, 2024

This PR addressed open comments from PR #36

  • renamed dirtyStateHandler to dirtyStateConfig
  • used IsDirtyHandlingEnabled() in the runMigrations function instead of explicitly checking if the dirty state handling is enabled
  • updated comments to use destination dir
  • used https://pkg.go.dev/testing#T.TempDir in the UTs added

Copy link

@daniel-garcia daniel-garcia left a comment

Choose a reason for hiding this comment

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

The code changes look fine. The commit messages and PR title needs to be descriptive of what is being changed.

migrate.go Show resolved Hide resolved
migrate.go Outdated Show resolved Hide resolved
migrate_test.go Outdated Show resolved Hide resolved
migrate_test.go Outdated Show resolved Hide resolved
migrate.go Show resolved Hide resolved
@venkat-iblox venkat-iblox changed the title Address PR comments from #36 Minor code updates from missed open comments to the migratedown feature Oct 9, 2024
@venkat-iblox venkat-iblox changed the title Minor code updates from missed open comments to the migratedown feature Minor code updates from missed open comments from #36 to the migratedown feature Oct 9, 2024
@akleinib
Copy link

akleinib commented Oct 9, 2024

All my comments have been addressed However it doesn't seem that changes related to Docker and build pertain to this PR.

@venkat-iblox
Copy link
Author

All my comments have been addressed However it doesn't seem that changes related to Docker and build pertain to this PR.

Thank you, I have removed the files for building a local image for testing. We probably should add a few of these paths into .gitignore.

@venkat-iblox venkat-iblox merged commit 5d974f1 into infobloxopen:ib Oct 9, 2024
2 of 5 checks passed
@venkat-iblox venkat-iblox deleted the pr-comments branch October 9, 2024 17:04
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.

3 participants