Skip to content

Notify#7

Merged
tertsdiepraam merged 27 commits into
mainfrom
notify
Dec 2, 2024
Merged

Notify#7
tertsdiepraam merged 27 commits into
mainfrom
notify

Conversation

@tertsdiepraam

@tertsdiepraam tertsdiepraam commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

This PR includes:

  • The implementation of ldns-notify and dnst notify.
  • Stelline in the env.
  • Printing version and help from ldns argument parsing.

@tertsdiepraam
tertsdiepraam marked this pull request as ready for review October 24, 2024 11:30

@mozzieongit mozzieongit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looked like you're mostly done, so I checked it out. There's a left-over TODO though.

Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/mod.rs Outdated
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs
Comment thread src/commands/notify.rs
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated
@mozzieongit

Copy link
Copy Markdown
Member

FYI: ldns-notify supports -v

@tertsdiepraam
tertsdiepraam marked this pull request as draft November 20, 2024 10:06
@tertsdiepraam
tertsdiepraam marked this pull request as ready for review November 22, 2024 14:44
@ximon18 ximon18 mentioned this pull request Nov 26, 2024

@bal-e bal-e left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really well done, @tertsdiepraam! This is some complex code but I like the way you've managed to get through it!

Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs
Comment thread src/commands/notify.rs
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated

@bal-e bal-e left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nicely done, @tertsdiepraam!

@mozzieongit mozzieongit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Last complaints, I swear! ... maybe :D

Comment thread src/commands/notify.rs Outdated
Comment thread src/commands/notify.rs Outdated
@tertsdiepraam

Copy link
Copy Markdown
Contributor Author

@mozzieongit do your worst! Give me that great feedback! 😄

@mozzieongit mozzieongit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work 🎉

@tertsdiepraam
tertsdiepraam merged commit 9432f48 into main Dec 2, 2024
@tertsdiepraam
tertsdiepraam deleted the notify branch December 2, 2024 13:22
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.

4 participants