artefactual / automation-tools

Tools to aid automation of Archivematica and AtoM.
GNU Affero General Public License v3.0
46 stars 33 forks source link

WIP Add create_avalon_dip script #107

Closed ablwr closed 5 years ago

ablwr commented 5 years ago

This is connected to https://github.com/archivematica/Issues/issues/643.

Explanation of what this does is in the issue above and also commented alongside.

For some explanation towards my choices for the purpose of code review: I think if this was going into the main code base, I would have extended this out into smaller functions that group together. But for me, the ethos of this repository is for people to read and customize things as they see fit, and I think it's more straightforward to keep it in one lengthy function for the use-case of people wanting to understand how and what this script does but are maybe not proficient in Python.

ablwr commented 5 years ago

@jraddaoui any chance you can counter-review this? It's only a slight modification of the create_dip.py script that you just updated (thank you).

ablwr commented 5 years ago

Hey @jraddaoui -- that makes sense to me and was my preferred option, but when this script was written a month or two ago, it was unclear how much create_dip would be changing in light of the CCA work and within the context of other people using it that we don't know about (which speaks to some of my frustration about the automation-tools lack of semver, for example). I also initially planned to perform create_dip and then build a script that moves things around in the Avalon-requested structure, but it seemed like it was doing unnecessary work.

I'll ping you on Slack to talk about how this relates to two client projects, but thanks for reviewing and I appreciate all the comments! I will make the changes some sort of way when we come up with a solution.

ablwr commented 5 years ago

I am going to close this PR based on the create_dip changes. I incorporated this script into the new create_dip and will open a subsequent PR.