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

Add absolute_path function #1121

Merged
merged 12 commits into from
Mar 3, 2022
Merged

Add absolute_path function #1121

merged 12 commits into from
Mar 3, 2022

Conversation

Laura7089
Copy link

I asked on the discord and was told there was no function to get the absolute path from a relative path, so here one is!

I haven't written tests for this, please let me know what form the should take.

Laura7089
Laura7089 commented Feb 27, 2022
View reviewed changes
src/function.rs Outdated Show resolved Hide resolved
casey
casey reviewed Feb 27, 2022
View reviewed changes
src/function.rs Outdated Show resolved Hide resolved
casey
casey reviewed Feb 27, 2022
View reviewed changes
src/function.rs Outdated Show resolved Hide resolved
casey
casey requested changes Feb 28, 2022
View reviewed changes
Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

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

Some comments, also this needs tests. New tests should be written using the Test struct, and not the test! macro. There are some examples in tests/functions.rs, where tests for this function could also go.

src/function.rs Outdated Show resolved Hide resolved
src/function.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@Laura7089 Laura7089 requested a review from casey February 28, 2022 15:37
@Laura7089 Laura7089 changed the title Add abs_path function Add absolute_path function Feb 28, 2022
casey
casey reviewed Mar 1, 2022
View reviewed changes
src/function.rs Outdated Show resolved Hide resolved
tests/functions.rs Show resolved Hide resolved
src/function.rs Outdated Show resolved Hide resolved
casey
casey requested changes Mar 1, 2022
View reviewed changes
Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

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

Getting there!

Laura Demkowicz-Duffy and others added 2 commits March 1, 2022 03:42
@casey casey enabled auto-merge (squash) March 2, 2022 23:34
casey
casey reviewed Mar 2, 2022
View reviewed changes
tests/functions.rs Outdated Show resolved Hide resolved
@casey casey self-requested a review March 3, 2022 00:00
@casey casey disabled auto-merge March 3, 2022 00:00
@casey casey enabled auto-merge (squash) March 3, 2022 00:00
@casey
Copy link
Owner

casey commented Mar 3, 2022

For some reason the CI job didn't start, so I pushed an empty commit. This should merge soon. If this is functionality that you need, I'm happy to cut a release, just let me know.

@Laura7089
Copy link
Author

Laura7089 commented Mar 3, 2022 •
edited
Loading

I'm in no desperate need :)

This has been my first contribution to a FOSS project, so thanks for giving me the opportunity and helping me through it!

@casey casey merged commit a3f61a1 into casey:master Mar 3, 2022
@casey
Copy link
Owner

casey commented Mar 3, 2022

Nice! Very glad to hear it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@casey casey casey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Laura7089 @casey