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

Patches for a new motd emitter #174

Closed
wants to merge 1 commit into from

Conversation

kushaldas
Copy link
Contributor

This is following https://bugzilla.redhat.com/show_bug.cgi?id=995537. In future we can decide on the motd templates as nothing exists for now.

@jsilhan
Copy link
Contributor

jsilhan commented Oct 17, 2014

Looks good aside punctuation :). Can you add some test using mock to see if the message was written into /etc/motd, please?

@@ -33,7 +33,7 @@ Alternative CLI to ``dnf upgrade`` with specific facilities to make it suitable

The operation of the tool is completely controlled by the configuration file and the command only accepts single optional argument pointing to it. If no configuration file is passed from the command line, ``/etc/dnf/automatic.conf`` is used.

The tool synchronizes package metadata as needed and then checks for updates available for the given system and then either exits, downloads the packages or downloads and applies the packages. The outcome of the operation is then reported by a selected mechanism, for instance via the standard output or email.
The tool synchronizes package metadata as needed and then checks for updates available for the given system and then either exits, downloads the packages or downloads and applies the packages. The outcome of the operation is then reported by a selected mechanism, for instance via the standard output or email or motd messages.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...standard output, email or motd messages., please

@jsilhan
Copy link
Contributor

jsilhan commented Oct 17, 2014

This looks good WRT code. Flush all the commits into 1 (it's still only one feature) with the message Adds new motd emitter for dnf-automatic (RhBug:995537), please, to have it easily tracked for the next release notes.

@ghost
Copy link

ghost commented Oct 17, 2014

Did you run Py3 tests? I bet that the mock_open should raise NameError: name 'file' is not defined. I also think that in Python 3, the unittest.mock_open function should be used instead.

@jsilhan
Copy link
Contributor

jsilhan commented Oct 20, 2014

Merged, thanks.

@jsilhan jsilhan closed this Oct 20, 2014
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.

None yet

2 participants