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

Add "Complete!" message after succesfull transaction #1553

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

j-mracek
Copy link
Contributor

Keeping the last message something abou scriptlets is not clear message that everything passed correctly and program was terminated correctly. The commit uses the same message like DNF4. Contrary to DNF4, this commit does not add Complete! message when there is anything to perform - no transaction table.

Closes: #613

@jrohel jrohel self-assigned this Jun 20, 2024
dnf5/main.cpp Outdated
@@ -1444,6 +1444,11 @@ int main(int argc, char * argv[]) try {

log_router.info("DNF5 finished");

// Print Complete! message only when transaction is created to prevent poluting output from repoquery or other command
if (auto * transaction = context.get_transaction(); transaction && !transaction->empty()) {
std::cout << "Complete!\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to print the message also in quiet (-q) mode? If not, use context.print_info("Complete"); instead of printing directly to std::cout.

In dnf5 quiet mode only partially works. For example, transaction table and transaction progress is visible even in quiet mode. We probably want to fix that as well. But that's out of the scope of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I am changing it to react to -q.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anyway scriptlets are still visible in -q mode and even worse, the new line is not added after the last entry.

@ppisar
Copy link
Contributor

ppisar commented Jun 21, 2024

I hope the message does not pollute stdout of "dnf5 repoquery" output.

@jan-kolarik
Copy link
Member

I hope the message does not pollute stdout of "dnf5 repoquery" output.

// Print Complete! message only when transaction is created to prevent poluting output from repoquery or other command

It seems, it should not 🙂

dnf5/main.cpp Outdated Show resolved Hide resolved
dnf5/main.cpp Outdated
@@ -1444,6 +1444,11 @@ int main(int argc, char * argv[]) try {

log_router.info("DNF5 finished");

// Print Complete! message only when transaction is created to prevent poluting output from repoquery or other command
if (auto * transaction = context.get_transaction(); transaction && !transaction->empty()) {
std::cout << "Complete!\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

I find the exclamation mark for an information message inappropriate. Could we use "Complete.\n" instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no preferences. "Complete!" was used by DNF4.

Keeping the last message something abou scriptlets is not clear
message that everything passed correctly and program was terminated
correctly. The commit uses the same message like DNF4. Contrary
to DNF4, this commit does not add Complete! message when there is
anything to perform - no transaction table.

Closes: rpm-software-management#613
@jrohel
Copy link
Contributor

jrohel commented Jun 21, 2024

Thanks.

@jrohel jrohel added this pull request to the merge queue Jun 21, 2024
Merged via the queue into rpm-software-management:main with commit 89ccf36 Jun 21, 2024
14 of 15 checks passed
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.

Add message at the end of process?
4 participants