Skip to content

Add functions rust#59

Open
uchitsa wants to merge 11 commits into
yegor256:masterfrom
uchitsa:functions-rust
Open

Add functions rust#59
uchitsa wants to merge 11 commits into
yegor256:masterfrom
uchitsa:functions-rust

Conversation

@uchitsa

@uchitsa uchitsa commented Oct 4, 2024

Copy link
Copy Markdown

for issue #9

@yegor256

yegor256 commented Oct 5, 2024

Copy link
Copy Markdown
Owner

@uchitsa thanks for the contribution! The code looks good, but some CI workflows don't pass. Please, check.

@uchitsa

uchitsa commented Oct 7, 2024

Copy link
Copy Markdown
Author

@yegor256 let's try the workflows again please

@uchitsa

uchitsa commented Oct 7, 2024

Copy link
Copy Markdown
Author
Run aws-actions/configure-aws-credentials@v4
Error: Credentials could not be loaded, please check your action inputs: Could not load credentials from any providers

@yegor256 what should I do for fix it?

@yegor256

yegor256 commented Oct 8, 2024

Copy link
Copy Markdown
Owner

@uchitsa try to push to the branch one more time, you should see the output of a new CI job I just added, it will help

@uchitsa

uchitsa commented Oct 8, 2024

Copy link
Copy Markdown
Author

@yegor256 FYI PR returns 404 for me. I try to push to the branch.

@yegor256

yegor256 commented Oct 9, 2024

Copy link
Copy Markdown
Owner

@uchitsa the CI is still broken, see the output of the local job

@kreinba kreinba left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Requesting changes on one correctness defect at rust/functions.rs:39. The new fibo returns 1 for x<2, which shifts the sequence by one position relative to cpp/functions.cpp and rust/recursion.rs, both of which return x so fib(0)=0. The whole repo compares implementations of the same algorithm across languages, so this file needs to produce the same numbers. The matrix file mirrors the cpp version correctly. Inline comment below.

Comment thread rust/functions.rs Outdated
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.

3 participants