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

added test for site set #70

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Sampriti-Mitra
Copy link

@Sampriti-Mitra Sampriti-Mitra commented Oct 10, 2021

Checklist

Please note that in case of below checklist not updated accordingly, the maintainers have the right to immediately close your pull request.

  • You created a branch from develop branch.
  • You PR is raised on develop branch and not onmain.
  • You have read the Contribution Guidlines before creating this PR.

Description of the Change

Alternate Designs

Risk Impact

Verification Process

Release Notes

@@ -15,6 +15,7 @@ require (
github.com/rs/cors v1.7.0
github.com/shirou/gopsutil v3.21.4+incompatible
github.com/spf13/cobra v1.1.3
github.com/stretchr/testify v1.3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

1.3.0 is kinda old, the latest version is 1.7.0

}

func removeFile(baseFileName string) {
err := os.RemoveAll(baseFileName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Your indentation is inconsistent, you might want to run gofmt

@@ -0,0 +1,164 @@
package site

import (
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice to run goimports and let it order the imports by

  1. Standard packages
  2. External packages
  3. Private packages

assert.Equal(t, tt.want, output)

removeFile(baseFileName)

Copy link
Contributor

Choose a reason for hiding this comment

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

spurious newline

}

func TestSetSite(t *testing.T) {

Copy link
Contributor

Choose a reason for hiding this comment

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

spurious newline

fileName := filepath.Join(baseFileName, tt.args["pathToSiteInfo"].(string))

dest, err := os.Create(fileName)

Copy link
Contributor

Choose a reason for hiding this comment

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

it would be better style to stick the line where err is instantiated with the line where the error is checked

@mohammed786
Copy link
Collaborator

Hi @Sampriti-Mitra
Thanks for contributing. Your PR is accepted for the Hacktoberfest. Please fill this form to become eligible for LoginRadius cool swags. :)

@mohammed786 mohammed786 self-requested a review October 29, 2021 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants