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

Implementation of puppy store #679

Merged
merged 10 commits into from
Jul 29, 2020
Merged

Implementation of puppy store #679

merged 10 commits into from
Jul 29, 2020

Conversation

jimbosoft
Copy link
Collaborator

@jimbosoft jimbosoft commented Oct 1, 2019

CRUD interface and storage using map and sync.map

Fixes #680

Review of colleague's PR #605 and #533

Changes proposed in this PR:

Implementation of the puppy CRUD interface and storing it in map and sync.map

-

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #679 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #679   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         288    288           
  Lines        5967   5967           
=====================================
  Hits         5967   5967
Impacted Files Coverage Δ
11_notify/n0npax/pkg/puppy/errors.go 100% <ø> (ø) ⬆️
05_stringer/alextmz/main.go 100% <100%> (ø) ⬆️
07_errors/alextmz/errors.go 100% <100%> (ø) ⬆️
08_project/alextmz/pkg/puppy/store/syncstore.go 100% <100%> (ø) ⬆️
06_puppy/alextmz/syncstore.go 100% <100%> (ø) ⬆️
11_notify/n0npax/cmd/puppy-server/main.go 100% <100%> (ø) ⬆️
02_bubble/alextmz/main.go 100% <100%> (ø) ⬆️
07_errors/alextmz/syncstore.go 100% <100%> (ø) ⬆️
07_errors/alextmz/main.go 100% <100%> (ø) ⬆️
07_errors/alextmz/mapstore.go 100% <100%> (ø) ⬆️
... and 6 more

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 a3da861...efffc14. Read the comment docs.

Copy link
Contributor

@camscale camscale left a comment

Choose a reason for hiding this comment

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

Just a few comments, less than I usually make. Some things I commented on occurs multiple times and I did not comment on each occurrence, so do check the rest of the code to apply the same comments elsewhere.

06_puppy/jimbotech/mapStore.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/mapStore.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/mapStore.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/mapStore.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/mapStore.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/mapStore_test.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/mapStore_test.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/mapStore_test.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/puppy.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/puppy_test.go Outdated Show resolved Hide resolved
camscale
camscale previously approved these changes Oct 30, 2019
Copy link
Contributor

@camscale camscale left a comment

Choose a reason for hiding this comment

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

Minor comments. You know what you're doing here, so i'll approve for merging. You can choose to address the comments or not.

06_puppy/jimbotech/sync_mapstore.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/sync_mapstore.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/sync_mapstore.go Outdated Show resolved Hide resolved
06_puppy/jimbotech/storer_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@camscale camscale left a comment

Choose a reason for hiding this comment

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

LGTM

@jimbosoft jimbosoft merged commit cf576ff into anz-bank:master Jul 29, 2020
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.

Lab 6 - CRUD puppy with interface
2 participants