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

Added end line in printf #5489

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

yael-tramier
Copy link

What does this PR aim to accomplish?:

There was a missing end line in the basic-install script.

How does this PR accomplish the above?:

I added it.

Link documentation PRs if any are needed to support this PR:


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)
  6. I have checked that another pull request for this purpose does not exist.
  7. I have considered, and confirmed that this submission will be valuable to others.
  8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  9. I give this submission freely, and claim no ownership to its content.

  • I have read the above and my PR is ready for review.

Signed-off-by: Yaël TRAMIER <tramier.yael@gmail.com>
@DL6ER
Copy link
Member

DL6ER commented Nov 17, 2023

Why do you think there should be a \n here?

@yael-tramier
Copy link
Author

Hello, at the end of the installation, there is : [i] Restarting pihole-FTL service...admin@pihole:/tmp#

@yubiuser
Copy link
Member

When this is the last line printed, something is wrong with the installation. It likely fails to restart FTL

@DL6ER
Copy link
Member

DL6ER commented Nov 18, 2023

The solution should rather be to capture this error and show something useful (like systemctl status)

@dfelton
Copy link

dfelton commented Jan 15, 2024

The solution should rather be to capture this error and show something useful (like systemctl status)

First comment in this repo else I'd offer to do this for you @DL6ER.

Might I suggest applying this statement as a comment on the one file changed. This would then cause it to be a "Conversation" recognized by the pull request's auto checks that must be resolved, and it would no longer appear as though this PR has met all checks and is merely awaiting an approved review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants