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

Always include the IPAddressHint in the noderef WIP #859

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

Conversation

ArneBab
Copy link
Contributor

@ArneBab ArneBab commented Sep 9, 2023

This is a stab at addressing a user-report from FMS that accepting the temporary IP override took 3 ARK uploads.

if((!hadAddedValidIP || confidence <= 2 || hasIPAddressHint)
&& oldIPAddress != null
&& !oldIPAddress.equals(overrideIPAddress)
&& !addresses.contains(oldIPAddress)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Indent :)

@Bombe
Copy link
Contributor

Bombe commented Feb 10, 2024

@ArneBab, what did I tell you about creating pull requests before you’re actually done with something? 😁

@ArneBab
Copy link
Contributor Author

ArneBab commented Feb 11, 2024

@ArneBab, what did I tell you about creating pull requests before you’re actually done with something? 😁

This has the "forReviewDontMerge" tag for a reason ☺

@Bombe
Copy link
Contributor

Bombe commented Feb 12, 2024

@ArneBab, what did I tell you about creating pull requests before you’re actually done with something? 😁

This has the "forReviewDontMerge" tag for a reason ☺

😡

And I think I already told you the last time you said this that the tag does not prevent an email to be sent. Everybody who is subscribed to this repository because they want to keep up to date with it gets an email about somebody doesn’t know how not to create a pull request for everything they do.

Seriously. NOT creating a pull request is way easier than adding a tag. For everybody involved.

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