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

Cleanup - Initial Project Refactor #3

Merged
merged 5 commits into from
Jul 31, 2024
Merged

Conversation

ssulei7
Copy link
Contributor

@ssulei7 ssulei7 commented Jul 30, 2024

  • Updated go.mod to reflect source repository name
  • Moved commit remap logic from cmd/root.go into internal/commitremap. Chances are some functions around file processing can be broken out of here as well, but we'll see.
  • Removed func main, and did an inline function for the root cmd. I didn't see much of a point of adding a concrete function for the root command at this point in time 🤷🏽
  • Added initial unit test around parsing commit map
  • Added unit test execution as part of CI workflow

@ssulei7 ssulei7 requested review from kuhlman-labs and amenocal July 30, 2024 18:20
@ssulei7
Copy link
Contributor Author

ssulei7 commented Jul 30, 2024

@amenocal @kuhlman-labs please do an integration test and review the above. I have a bit more changes incoming regarding unit tests, but getting this initial PR out there. :)

}
}

func replaceSHA(data interface{}, oldSHA string, newSHA string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a later PR, I would like to rework this function as I'm getting super spooked around the usage of empty interfaces here (e.g., any type)

Copy link
Contributor

Choose a reason for hiding this comment

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

@ssulei7 good catch!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed

@kuhlman-labs
Copy link
Collaborator

@ssulei7 LGTM. My integration tests passed, I just added one commit to add in the issue_events file to be processed as well.

@ssulei7 ssulei7 merged commit f40a2e9 into main Jul 31, 2024
1 check passed
@ssulei7 ssulei7 deleted the ssulei7/initial-project-refactor branch July 31, 2024 01:46
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