[Merge] lp:~thomas-voss/trust-store/add_convenience_function_for_processing_incoming_requests into lp:trust-store

Thomas Voß thomas.voss at canonical.com
Thu Jul 17 15:39:52 UTC 2014


> So I think this all looks pretty good, here are just a few more observations:
> 
> 565     +        static std::string s{"com.ubuntu.trust.store"};
> 
> I know it doesn't really matter but the "const" is missing here.
> Should be: "static const std::string s{"com.ubuntu.trust.store"};"
> 

Fixed.

> 1493    +            (core::trust::mir::cli::option_title,
> boost::program_options::value<std::string>(), "Title of the prompt");
> 
> Missing a full-stop at the end of the string (doesn't match the others).
> 

Fixed.

> 1530    +        default:
> 1531    +            break;
> 
> Redundant "break;"

But the compiler needs it :)
> 
> 2514    +std::shared_ptr<core::trust::Store::Query> a_null_query()
> 

Keeping it for future convenience.

> 2052    +std::function<core::trust::Request::Answer(const
> core::posix::wait::Result&)> mock_translator_to_functor(const
> std::shared_ptr<MockTranslator>& ptr)
> 

Removed. 

> 2062    +std::shared_ptr<core::trust::mir::PromptProviderHelper>
> a_prompt_provider_calling_bin_false()
> 

Removed. 

> 2080    +std::shared_ptr<core::trust::mir::PromptProviderHelper>
> a_prompt_provider_calling_bin_true()
> 

Removed.

> The above methods don't appear in your tests.
> 
> 1426    === added file 'src/core/trust/mir/prompt_main.cpp'
> 2786    === added file 'tests/test_prompt.cpp'
> 
> These 2 files are pretty much doing the same thing. I get how one is a test
> version of the other but is it not a maintenance nightmare waiting to happen?
> If the app is used for internal use only, why not just check for something
> like a --testargs argument when running testing.

Fixed.
-- 
https://code.launchpad.net/~thomas-voss/trust-store/add_convenience_function_for_processing_incoming_requests/+merge/226696
Your team Ubuntu Phablet Team is subscribed to branch lp:trust-store.



More information about the Ubuntu-reviews mailing list