repobee / repobee-sanitizer

A plugin for sanitizing master repositories before distribution to students
MIT License
2 stars 3 forks source link

Add `--discover-files`, mutually exclusive to `--file-list` #32

Closed slarse closed 4 years ago

slarse commented 4 years ago

Instead of explicitly specifying which files to sanitize with a file list, the -d|--discover-files flag should search through all files in the repository for sanitizer blocks. It must be mutually exclusive to --file-list.

tohanss commented 4 years ago

How will this work with #17? The logical way I can think of is that you only want it to yell if you have specified a file to sanitize and there is nothing to sanitize. When you want it to sanitize everything it can find, it doesn't matter, and therefore it shouldn't yell

slarse commented 4 years ago

@tohanss We should not attempt to sanitize a file that does not have markers, and so this functionality is orthogonal to #17. The point of discovery is to find the files to sanitize, and that should only be files that contain markers. If somehow we still try to sanitize a file without markers, then there has been a mistake in file discovery, and #17 still applies.

To give you a better idea, discovery/dictate should look something like this on a high level, in _sanitize_repo:

    def _sanitize_repo(
        self, args: argparse.Namespace, api: None,
    ) -> Optional[Mapping[str, List[plug.Result]]]:
        message = _check_repo_state(args.repo_root)
        if message:
            return plug.Result(
                name="sanitize-repo", msg=message, status=plug.Status.ERROR,
            )

        # The only difference between discovery and dictate is how the relpaths are determined
        file_relpaths = (
            _parse_file_list(args.file_list) if args.file_list
            else _discover_dirty_files(args.repo_root)
        )

        if args.no_commit:
            LOGGER.info("Executing dry run")
            _sanitize_files(args.repo_root, file_relpaths)
        else:
            LOGGER.info(f"Sanitizing repo and updating {args.target_branch}")
            _sanitize_to_target_branch(
                args.repo_root, file_relpaths, args.target_branch
            )