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

Improve usability via iptc.easy #269

Merged
merged 13 commits into from
Apr 26, 2019
Merged

Improve usability via iptc.easy #269

merged 13 commits into from
Apr 26, 2019

Conversation

jllorente
Copy link
Collaborator

[do not merge]
Placeholder for addressing the issue discussed #268
This PR contains a first version of the ported iptc_helper developed earlier.
It adds new functionality regarding get/set policy.
It should obsolete PR #222 regarding the dictionary functionality, as we now expose encode../decode.. functions in this new module.

@ldx
Copy link
Owner

ldx commented Apr 8, 2019

Looks good. Can you also:

  • add tests for the public functions and
  • update the README?

As for the README update, I think it'd be useful to add a short explanation on the use case (higher level than the current library, easier to use for short scripts) and a few simple examples?

@jllorente
Copy link
Collaborator Author

Regarding the tests, anything else than the decode() encode() functions that translate betwen iptc_rules to dictonaries would require admin rights to run, that's why I decided not to include them.
And for the README section, perhaps we could include the full list with the supported functions in the easy module instead?
@ldx, let me know what you think

@jllorente
Copy link
Collaborator Author

Regarding the tests, anything else than the decode() encode() functions that translate betwen iptc_rules and dictionaries would require admin rights to run, that's why I decided not to include them.
And for the README section, perhaps we could include the full list with the supported functions in the easy module instead?
@ldx, let me know what you think

@ldx
Copy link
Owner

ldx commented Apr 19, 2019

@jllorente it's okay if not every function from the easy module is documented in the README, users can check out the module itself.

I think this is good to merge, anything else you'd like to add?

@jllorente
Copy link
Collaborator Author

Alright then, let's merge it at this time so people are able to start using it. Hopefully we'll get some feedback on how to improve it!

@ldx ldx merged commit f8a6641 into ldx:master Apr 26, 2019
@ldx
Copy link
Owner

ldx commented Apr 26, 2019

Thanks! I've also added you as a collaborator of the project. LMK if you have any questions.

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.

2 participants