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

Refactor: change set simplification/removal --> cleaner design, more flexible #27

Closed
sotte opened this issue Jul 25, 2013 · 2 comments
Closed

Comments

@sotte
Copy link
Contributor

sotte commented Jul 25, 2013

Catkinize offers this "changeset" or "interactive mode" feature in which it basically prints a big list of all changes. I like the idea but in my experience it is not really too useful. The list is normally too big (especially when you catkinize a stack) and you should verify the changes (probably with git, your favorite diff tool) anyway.

Also the changeset approach obfuscates what really happens. Depending on if file names in the changeset exist (and/or the content) you backup/delete/change files. This is really implicit and, as you know, "explicit is better than implicit" :)

Another point is, that the description of what is going to happen (prompt_changes()) and the logic is divided. The logic itself is also divided into create_changeset() and perform_changes().

So I propose a simplified method outlined below:
(I know it must change a bit for catkinize_stack.)

possible = handle_manifest(dryrun=True) and handle_cmake(dryrun=True) and handle_make(dryrun=True)

if possible and confirm('continue?'):
    handle_manifest(dryrun=False)
    handle_cmake(dryrun=False)
    handle_make(dryrun=False)

handle_X return True if it can perform all actions. In dryrun-mode, handle_X does not perform any file operations and therefore is side effect free.

Also the handle_X methods are more flexible that the current changeset-approach.

What do you think?

@sotte
Copy link
Contributor Author

sotte commented Aug 5, 2013

Here are the first commits: https://github.com/sotte/catkinize/tree/simplification_changeset

@sotte
Copy link
Contributor Author

sotte commented Aug 12, 2013

Closing this one for issue #37.

@sotte sotte closed this as completed Aug 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant