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

Propagate default_vhost to zabbix::web #826

Merged
merged 3 commits into from
Dec 7, 2022
Merged

Conversation

ikcalB
Copy link
Contributor

@ikcalB ikcalB commented Jul 27, 2022

Pull Request (PR) description

enables workaround for #825, fixes bug

This Pull Request (PR) fixes the following issues

parameter was not beeing propagated to zabbix::web, therefore rendering it
useless.

enables workaround for voxpupuli#825, fixes bug
parameter was not beeing propagated to zabbix::web, therefore rendering it
useless.
@ikcalB
Copy link
Contributor Author

ikcalB commented Jul 28, 2022

I don't think the PR change can cause errors on some tests, but succeed on most.

Skimping through the reasons why it failed, I guess the tests may need checking? Or maybe the master branch has some issues.
Maybe someone with more experience can chime in on this?

@ikcalB
Copy link
Contributor Author

ikcalB commented Sep 22, 2022

@pccibot I think the tests might fail because of something other than the PR. (one liner. if working on any version, should work on all)

can you please comment?

@smortex
Copy link
Member

smortex commented Sep 22, 2022

Recent PR have passing CI, I guess the error you see have been fixed :-). Can you please rebase your changes on top of the main branch so that we can sort this out?

From your working directory:

git fetch origin         # Download the latest code we have here
git rebase origin/master # Move your commits on top of the main branch
git push -f              # Send the changes (-f is required because we re-wrote history)

@smortex smortex added the bug Something isn't working label Sep 22, 2022
@ikcalB
Copy link
Contributor Author

ikcalB commented Sep 25, 2022

Hi!
Will do that, thanks.

Am on vacation, will take 10 days, approx.

@ikcalB
Copy link
Contributor Author

ikcalB commented Nov 10, 2022

@smortex please have a look at the static validation checks AND the diff: I added a single line, in init.pp@354 and static checks are complaining about opening whitespace error at web.pp (i.e. 410, and 4 more locations in that file)

TL;DR: static checks complain about upstream, not my change

@smortex
Copy link
Member

smortex commented Dec 6, 2022

Can you please rebase your changes on top of the main branch (CI should be fixed)?

From your working directory:

git fetch origin       # Download the latest code we have here
git rebase origin/master # Move your commits on top of the main branch
git push -f            # Send the changes (-f is required because we re-wrote history)

rebase on current master
@ikcalB
Copy link
Contributor Author

ikcalB commented Dec 7, 2022

@smortex looks like the checks are passing now, thanks.

Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM!

@smortex smortex merged commit 7a0bed1 into voxpupuli:master Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants