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 router nokia-g240wb #791

Merged

Conversation

elulcao
Copy link
Contributor

@elulcao elulcao commented Oct 12, 2022

  • What does this PR aim to accomplish?:

Added Router Nokia G-240W-B guide

  • How does this PR accomplish the above?:

Added Router Nokia G-240W-B guide for IPv4 and IPv6 addresses

  • What documentation changes (if any) are needed to support this PR?:

Added docs/routers/nokia-G240WB.md

Screen Shot 2022-10-12 at 1 10 44 PM


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)

  • [ x ] I have read the above and my PR is ready for review. Check this box to confirm

@netlify
Copy link

netlify bot commented Oct 12, 2022

Deploy Preview for pihole-docs ready!

Name Link
🔨 Latest commit 1cda001
🔍 Latest deploy log https://app.netlify.com/sites/pihole-docs/deploys/649d991611a1970008a51685
😎 Deploy Preview https://deploy-preview-791--pihole-docs.netlify.app/routers/nokia-g240wb
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Thanks for your submission. Before reviewing your PR in detail, could you please crop/compress+crop your images?

.gitignore Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Looking at your screenshots, your DHCP pool ranges from .50 - .250. However, your Pi's static address .100 is within the pool. Your router won't know this address is already taken and can hand out this address to some other device.
It's better to give devices static addresses outside of the DHCP pool to avoid this conflict. You should set your Pi's address to something <.50

docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Thanks. Now the screenshots don't fit the new IP of 192.168.1.10 anymore.

docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
@elulcao
Copy link
Contributor Author

elulcao commented Nov 7, 2022

Thanks. Now the screenshots don't fit the new IP of 192.168.1.10 anymore.

Updated the pics, thanks for the feedback:)

@DL6ER
Copy link
Member

DL6ER commented Jan 28, 2023

@yubiuser You reviewed this before, do you still have any objections?

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@elulcao elulcao force-pushed the elulcao/add-nokia-g240wb-router-guide branch from 7d78156 to 82d27af Compare February 8, 2023 03:16
@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2023

Conflicts have been resolved.

@elulcao
Copy link
Contributor Author

elulcao commented Mar 16, 2023

@yubiuser I addressed the comments, PR is ready to be reviewed or merged. Do you mind taking a look to it? Thanks in advance.

docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
@elulcao
Copy link
Contributor Author

elulcao commented Mar 17, 2023

@yubiuser thanks for the review, the guide now has the latest suggestion.

docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Show resolved Hide resolved
@elulcao
Copy link
Contributor Author

elulcao commented May 10, 2023

@yubiuser I addressed the last moments, this time the text should be better. Let me know if you want me to do another update to the Readme file.
Thanks.

docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
@elulcao
Copy link
Contributor Author

elulcao commented May 12, 2023

Hi @yubiuser thanks for the latest comments, I didn't notice the typo. The duplicate entry, I noticed the same but I was unsure if remove or not since it was chained to our previous conversations

Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

As (hopefully) last remark: I would re-order the structure of "Grab your IPv4 and IPv6 address from your Raspberry Pi"

And please squash down all your commits into a singel one.

docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
docs/routers/nokia-G240WB.md Outdated Show resolved Hide resolved
Signed-off-by: Daniel Carvallo <elulcao@icloud.com>
@elulcao elulcao force-pushed the elulcao/add-nokia-g240wb-router-guide branch from 02e789a to 3eb3077 Compare June 24, 2023 20:47
@elulcao
Copy link
Contributor Author

elulcao commented Jun 24, 2023

As (hopefully) last remark: I would re-order the structure of "Grab your IPv4 and IPv6 address from your Raspberry Pi"

And please squash down all your commits into a singel one.

Hello @yubiuser, hope your doing good!
I finally squashed and addressed the comments for this PR 👍 Could you share an +1?
Let me know if anything else is needed.

Signed-off-by: Daniel Carvallo <elulcao@icloud.com>
@yubiuser yubiuser merged commit fcf54e0 into pi-hole:master Jul 2, 2023
9 checks passed
@elulcao elulcao deleted the elulcao/add-nokia-g240wb-router-guide branch July 3, 2023 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants