Skip to content

Allows a logger to be given during cloning #473

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

Closed
wants to merge 1 commit into from

Conversation

sergio-bobillier
Copy link

Your checklist for this pull request

🚨 Please review the guidelines for contributing to this repository.

  • Ensure all commits include DCO sign-off.
  • Ensure that your contributions pass unit testing.
  • Ensure that your contributions contain documentation if applicable. (N/A)

Description

Currently, if you pass a logger to the clone method in the Base class it is completely ignored, not only it is not used during the cloning phase but the resulting Base instance doesn't have the logger attached to it, meaning that all subsequent operations over that repository are not logged.

This Pull Request attempts to remedy that by adding code to pass the logger object to the constructor of the Git::Lib class as well as to the constructor of the Base class itself.

In order to avoid coupling the classes (more than they already are) I decided to merge the logger to the Hash returned by the clone method of the Git::Lib class instead of passing it and then having the method return it in the resulting Hash.

@stale
Copy link

stale bot commented Aug 23, 2020

A friendly reminder that this issue had no activity for 60 days.

Modifies the clone method in the Base class to allow it to receive and
use a logger during the clonning phase as well as pass it down to the
class constructor so that the created instance has access to it as well.

Signed-off-by: Sergio Bobillier <sergio.bobillier@esrlabs.com>
@stale
Copy link

stale bot commented Jun 11, 2021

A friendly reminder that this issue had no activity for 60 days.

@stale stale bot added the stale label Jun 11, 2021
@jcouball
Copy link
Member

This was fixed in #501

@jcouball jcouball closed this Apr 22, 2022
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