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.

Sign up for GitHub

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

Jump to bottom

implementation #1

Merged
merged 2 commits into from
Nov 20, 2023
Merged

implementation #1

merged 2 commits into from
Nov 20, 2023

Conversation

evlli
Copy link
Contributor

@evlli evlli commented Nov 13, 2023

No description provided.

@evlli evlli self-assigned this Nov 13, 2023
@evlli
Copy link
Contributor Author

evlli commented Nov 13, 2023

currently draft because needs atleast some basic level of tests and currently the resolver in the config is not used, as I'm unsure how to do that best

jcgruenhage
jcgruenhage approved these changes Nov 13, 2023
View reviewed changes
Copy link
Member

@jcgruenhage jcgruenhage left a comment

Choose a reason for hiding this comment

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

First round of review, looking good in a lot of parts, but there's still a lot to do here in terms of error handling.

src/config.rs Show resolved Hide resolved
src/happy.rs Outdated Show resolved Hide resolved
src/happy.rs Outdated Show resolved Hide resolved
src/happy.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@jcgruenhage jcgruenhage self-requested a review November 13, 2023 09:59
jcgruenhage
jcgruenhage requested changes Nov 13, 2023
View reviewed changes
Copy link
Member

@jcgruenhage jcgruenhage left a comment

Choose a reason for hiding this comment

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

Misclicked, sorry, was supposed to be a "request changes".

@evlli
Copy link
Contributor Author

evlli commented Nov 16, 2023

CI is broked, see famedly/github-workflows#28

jcgruenhage
jcgruenhage requested changes Nov 16, 2023
View reviewed changes
src/cli.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
jcgruenhage
jcgruenhage requested changes Nov 16, 2023
View reviewed changes
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
@evlli evlli marked this pull request as ready for review November 17, 2023 11:36
jcgruenhage
jcgruenhage requested changes Nov 17, 2023
View reviewed changes
Copy link
Member

@jcgruenhage jcgruenhage left a comment

Choose a reason for hiding this comment

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

vendoring openssl or switching to ring for dnssec should fix our CI, can you try that?

Cargo.toml Outdated Show resolved Hide resolved
jcgruenhage
jcgruenhage approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@jcgruenhage jcgruenhage left a comment

Choose a reason for hiding this comment

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

lgtm!

@evlli evlli merged commit b5059f2 into main Nov 20, 2023
2 checks passed
@evlli evlli deleted the evlli/dev branch January 12, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcgruenhage jcgruenhage jcgruenhage approved these changes

@CherryKitten CherryKitten Awaiting requested review from CherryKitten

Assignees

@evlli evlli

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@evlli @jcgruenhage @lrsksr