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

show action initial implementation #25

Merged
merged 4 commits into from
Aug 24, 2017
Merged

show action initial implementation #25

merged 4 commits into from
Aug 24, 2017

Conversation

ndushay
Copy link
Contributor

@ndushay ndushay commented Aug 23, 2017

The idea was to get a very basic (crude, even) show action working so we could split out tickets to do additional work - I believe this PR achieves that.

I have indicated spots in the code that I know need more attention, and created github issues for them. Please leave a comment if you see more things that we need to change.

@SaravShah helped with this.

I included some other refactoring which I can split out into a separate PR if you deem it worth the trouble.

Resolves #12

@ndushay ndushay force-pushed the show-action-naomi branch 3 times, most recently from 156cbaf to 8958c87 Compare August 23, 2017 23:43
@coveralls
Copy link

Coverage Status

Coverage increased (+19.4%) to 56.41% when pulling 8958c87e56b6bf16601df8c6c43ef79d536b0b5f on show-action-naomi into 68fd879 on master.

@SaravShah
Copy link
Contributor

In the moab_storage_controller, Line 17 maybe change the name of the key to current_version?
Other than that, everything looks good to me!

@ndushay
Copy link
Contributor Author

ndushay commented Aug 24, 2017

So am I hearing that both @SaravShah and @jmartin-sul are in favor of renaming to current_version? If so, I will do that.

@ndushay
Copy link
Contributor Author

ndushay commented Aug 24, 2017

@jmartin-sul @SaravShah okay - I changed it to current_version at your suggestion. Thanks!

@sul-dlss sul-dlss deleted a comment from coveralls Aug 24, 2017
@sul-dlss sul-dlss deleted a comment from coveralls Aug 24, 2017
@sul-dlss sul-dlss deleted a comment from coveralls Aug 24, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+19.4%) to 56.41% when pulling beb3779 on show-action-naomi into 68fd879 on master.

@jmartin-sul jmartin-sul merged commit c9cb2a7 into master Aug 24, 2017
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

4 participants