-
Notifications
You must be signed in to change notification settings - Fork 31
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
Refactoring go version and many fixes for e2e tests #530
Conversation
end_to_end_testing/course_files/05_test_exit_on_post_install_hook.yml
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrew Suderman <andrew@sudermanjr.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things found when reviewing with Andy
|
||
"github.com/fairwindsops/reckoner/pkg/course" | ||
"github.com/fatih/color" | ||
"github.com/thoas/go-funk" | ||
) | ||
|
||
// Plot actually plots the releases | ||
func (c Client) Plot() error { | ||
func (c *Client) Plot() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirm why this was changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this because c.Continue()
uses a pointer to a client object. It doesn't seem to be a syntax error if we change it back to a non-pointer but I'm not really sure the repercussions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, it only needs to be a pointer if you're going to modify it inside the function. I've seen a general convention to just make a it a pointer though, so it's probably fine?
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Checklist
Description
What's the goal of this PR?
To fix most e2e tests for the go rewrite by adding the git repository
management feature and fixing some bugs that were uncovered by these
tests.
Also removed all python testing from the circleci config and temporarily
commented out the release workflow until we are ready to release the go
version.
What changes did you make?
What alternative solution should we consider, if any?