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

Renames #89

Merged
merged 5 commits into from
May 5, 2019
Merged

Renames #89

merged 5 commits into from
May 5, 2019

Conversation

DylanMeeus
Copy link
Contributor

@DylanMeeus DylanMeeus commented May 3, 2019

Hey Elliot,

I've done the renames that we discussed in #86
I altered the unit tests to reflect the changes in the names. All unit tests passed when I ran them, so I'm thinking this should work :)

~ Dylan


This change is Reviewable

@codecov-io
Copy link

codecov-io commented May 3, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@57366d4). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master    #89   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?      7           
  Lines             ?    783           
  Branches          ?      0           
=======================================
  Hits              ?    783           
  Misses            ?      0           
  Partials          ?      0
Impacted Files Coverage Δ
pie/float64s_pie.go 100% <100%> (ø)
pie/carpointers_pie.go 100% <100%> (ø)
pie/cars_pie.go 100% <100%> (ø)
pie/strings_pie.go 100% <100%> (ø)
pie/ints_pie.go 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57366d4...52da6e5. Read the comment docs.

Copy link
Owner

@elliotchance elliotchance left a comment

Choose a reason for hiding this comment

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

Reviewed 15 of 15 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @DylanMeeus)


functions/main.go, line 53 at r1 (raw file):

	{"Map", "map.go", ForAll},
	{"Unique", "unique.go", ForNumbersAndStrings},
	{"FilterNot", "filternot.go", ForAll},

These need to be sorted by the ​function name. You will also need to update the README.


functions/filternot.go, line 1 at r1 (raw file):

package functions

Please rename this file to filter_not.go

@DylanMeeus
Copy link
Contributor Author

Woops sorry,
I have just pushed the requested changes :)

Copy link
Owner

@elliotchance elliotchance left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 8 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Owner

@elliotchance elliotchance left a comment

Choose a reason for hiding this comment

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

Please fix up the merge conflict.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@DylanMeeus
Copy link
Contributor Author

Looks like it has no conflicts now :)

Copy link
Owner

@elliotchance elliotchance left a comment

Choose a reason for hiding this comment

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

Reviewed 13 of 13 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@elliotchance elliotchance merged commit bf85395 into elliotchance:master May 5, 2019
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