Merge lp:~elementary-apps/granite/document-contract-interface into lp:~elementary-pantheon/granite/granite

Proposed by Sergey "Shnatsel" Davidoff
Status: Merged
Approved by: Victor Martinez
Approved revision: 717
Merged at revision: 716
Proposed branch: lp:~elementary-apps/granite/document-contract-interface
Merge into: lp:~elementary-pantheon/granite/granite
Diff against target: 61 lines (+40/-0)
1 file modified
lib/Services/ContractorProxy.vala (+40/-0)
To merge this branch: bzr merge lp:~elementary-apps/granite/document-contract-interface
Reviewer Review Type Date Requested Status
Victor Martinez (community) Approve
Review via email: mp+214846@code.launchpad.net

Commit message

Documentation: Add descriptions for Granite.Services.Contract and Granite.Services.ContractorError.

Description of the change

Document Granite.Services.Contract interface to the extent of my knowledge: I know how Contractor works but I'm no good at Vala.

To post a comment you must log in.
716. By Sergey "Shnatsel" Davidoff

document ContractorError.SERVICE_NOT_AVAILABLE

Revision history for this message
Victor Martinez (victored) wrote :

Great work!

The description for the Contract interface isn't very accurate. As the API is quite self-describing, it'd be better to keep it simple. For example:

"Interface for accessing contract actions and properties." instead of "Opaque object representing a Contractor action".
We could simply avoid "Returned by" as it's rarely used in Vala documentation and you'd have to list every method of ContractorProxy returning such objects.

That said, you've done a great job documenting the methods of the Contract interface and you're also providing proper guidance on handling ContractorError.SERVICE_NOT_AVAILABLE.

review: Needs Fixing
717. By Sergey "Shnatsel" Davidoff

Improve description of Services.Contract interface according to Victor's guidance

Revision history for this message
Sergey "Shnatsel" Davidoff (shnatsel) wrote :

Fixed, thanks!

Is this good to go now?

Revision history for this message
Victor Martinez (victored) wrote :

Absolutely!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/Services/ContractorProxy.vala'
--- lib/Services/ContractorProxy.vala 2014-04-05 04:36:31 +0000
+++ lib/Services/ContractorProxy.vala 2014-04-09 00:03:36 +0000
@@ -20,11 +20,42 @@
20***/20***/
2121
22namespace Granite.Services {22namespace Granite.Services {
23 /**
24 * Interface for executing and accessing properties of Contractor actions
25 */
23 public interface Contract : Object {26 public interface Contract : Object {
27
28 /**
29 * Returns the display name of the contract, already internationalized
30 *
31 * @return The internationalized value of the 'Name' key in the .contract file.
32 * As of 2014, Contractor uses gettext to handle internationalization.
33 */
24 public abstract string get_display_name ();34 public abstract string get_display_name ();
35
36 /**
37 * Returns the description of the contract, already internationalized
38 *
39 * @return The internationalized value of the 'Description' key in the .contract file.
40 * As of 2014, Contractor uses gettext to handle internationalization.
41 */
25 public abstract string get_description ();42 public abstract string get_description ();
43
44 /**
45 * Returns an icon for this contract
46 *
47 * @return {@link Glib.Icon} based on the 'Icon' key in the .contract file.
48 */
26 public abstract Icon get_icon ();49 public abstract Icon get_icon ();
50
51 /**
52 * Executes the action on the given file
53 */
27 public abstract void execute_with_file (File file) throws Error;54 public abstract void execute_with_file (File file) throws Error;
55
56 /**
57 * Executes the action on the given list of files
58 */
28 public abstract void execute_with_files (File[] files) throws Error;59 public abstract void execute_with_files (File[] files) throws Error;
29 }60 }
3061
@@ -32,6 +63,15 @@
32 * thrown by {@link Granite.Services.ContractorProxy}63 * thrown by {@link Granite.Services.ContractorProxy}
33 */64 */
34 public errordomain ContractorError {65 public errordomain ContractorError {
66 /**
67 * Usually means that Contractor is not installed or not configured properly
68 *
69 * Contractor is not a compile-time dependency, so it is possible to
70 * install an application that uses it without installing Contractor.
71 *
72 * Upon receiving this error the application should disable its Contractor-related
73 * functionality, which typically means hiding the relevant UI elements.
74 */
35 SERVICE_NOT_AVAILABLE75 SERVICE_NOT_AVAILABLE
36 }76 }
3777

Subscribers

People subscribed via source and target branches