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

New Stack operations #44: isEmpty #67

Merged
merged 10 commits into from Apr 5, 2017
Merged

Conversation

jlr52
Copy link

@jlr52 jlr52 commented Apr 4, 2017

Hi fern4lvarez,

This pull request fixes the code reviews from #65

  1. Pull request merging to dev-0.2 instead of master
  2. change handler and api name to empty to conform naming convention
  3. Implement Empty function in stack.go
  4. Enforce 100% code test coverage.
  5. Allow maintainer to edit pull request.

Description:

Added isEmpty functionality to api. Now client can access is_empty rest call to
determine if the stack associated to the pass in stack id and db id is empty or not.

Testing Methodology:

Verify the following curl call will return is_empty = false

    curl -XPUT "localhost:1205/databases?name=MY_DATABASE"
    curl -XPUT "localhost:1205/databases/MY_DATABASE/stacks?name=BOOKSHELF"
    curl -XPOST localhost:1205/databases/MY_DATABASE/stacks/BOOKSHELF \
        -d '{"element":{"title":"1984","author":"George Orwell","ISBN":"1595404325","comments":[]}}'
    curl "localhost:1205/databases/MY_DATABASE/stacks/BOOKSHELF?is_empty"

Verify the following curl call will return is_empty = true

        curl -XPUT "localhost:1205/databases?name=MY_DATABASE"
        curl -XPUT "localhost:1205/databases/MY_DATABASE/stacks?name=BOOKSHELF"
        curl "localhost:1205/databases/MY_DATABASE/stacks/BOOKSHELF?is_empty"

Verify that empty db or stack will have standard empty response

jlr52 and others added 3 commits April 3, 2017 22:27
Description:

    Added isEmpty functionality to api. Now client can access is_empty rest call to
    determine if the stack associated to the pass in stack id and db id is empty or not.

Testing Methodology:

    Verify the following curl call will return is_empty = false

        curl -XPUT "localhost:1205/databases?name=MY_DATABASE"
        curl -XPUT "localhost:1205/databases/MY_DATABASE/stacks?name=BOOKSHELF"
        curl -XPOST localhost:1205/databases/MY_DATABASE/stacks/BOOKSHELF \
            -d '{"element":{"title":"1984","author":"George Orwell","ISBN":"1595404325","comments":[]}}'
        curl "localhost:1205/databases/MY_DATABASE/stacks/BOOKSHELF?is_empty"

    Verify the following curl call will return is_empty = true

            curl -XPUT "localhost:1205/databases?name=MY_DATABASE"
            curl -XPUT "localhost:1205/databases/MY_DATABASE/stacks?name=BOOKSHELF"
            curl "localhost:1205/databases/MY_DATABASE/stacks/BOOKSHELF?is_empty"

    Verify that empty db or stack will have standard empty response
@codecov-io
Copy link

codecov-io commented Apr 4, 2017

Codecov Report

Merging #67 into dev-0.2 will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           dev-0.2    #67    +/-   ##
=======================================
  Coverage      100%   100%            
=======================================
  Files           16     18     +2     
  Lines          657    778   +121     
=======================================
+ Hits           657    778   +121
Impacted Files Coverage Δ
pilad/conn.go 100% <100%> (ø) ⬆️
pila/stack.go 100% <100%> (ø) ⬆️
config/config.go 100% <0%> (ø) ⬆️
main.go
pkg/stack/stack.go 100% <0%> (ø)
pilad/config.go 100% <0%> (ø)
pilad/main.go 100% <0%> (ø)

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 de1a46f...c84f122. Read the comment docs.

@fern4lvarez
Copy link
Owner

This looks great now 👌

I added some small fixes, mostly formatting, wording and naming related. Then with a bit of time I'll try the functionality itself, update CHANGELOG and Merge ⭐

@jlr52 can you in the meantime update the pilad README.md file? Adding something like this, but for EMPTY? https://github.com/fern4lvarez/piladb/blob/master/pilad/README.md#get-databasesdatabase_idstacksstack_idsize

Thanks and great work!

@jlr52
Copy link
Author

jlr52 commented Apr 4, 2017

@fern4lvarez

For sure, I will update the docs on my next pull request. Thanks for fixing the coding styles and mismatch %v, I will be more meticulous next time.

@fern4lvarez
Copy link
Owner

@jlr52 you can do it in this same PR.

Description:

    Added documentation for EMPTY operation

Testing Methodology:

    n/a
@jlr52
Copy link
Author

jlr52 commented Apr 5, 2017

Doc is added.

@fern4lvarez fern4lvarez merged commit 04929aa into fern4lvarez:dev-0.2 Apr 5, 2017
@fern4lvarez
Copy link
Owner

Merged 🎉

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.

None yet

3 participants