search

ocaml-multicore / eio

Effects-based direct-style IO for multicore OCaml
Other
559 stars 72 forks source link

fix: test_symlink #768

Closed create2000 closed 1 month ago

create2000 commented 1 month ago

Worked on disabling the windows symlink.

create2000 commented 1 month ago

@patricoferris . This is the new PR

create2000 commented 1 month ago

@patricoferris -- I have done the fixes. Is this okay?

patricoferris commented 1 month ago

Definitely getting there @create2000.

I would recommend resetting this history and only (a) making the fix to test_symlink and adding the test_mkdirs into the tests of this file.

create2000 commented 1 month ago

Thank you @patricoferris -- I have added the test_mkdirs to the tests. About making the fix to test_symlink I understand it to mean resetting my git history to reflect fix: test_symlink. If this is it, I have done that.

patricoferris commented 1 month ago

Thanks @create2000 :))

mean resetting my git history to reflect fix: test_symlink

My point here is that right now, the diff is a little confusing. We have four distinct changes happening:

  1. The fix for the symlink test
  2. Adding test_mkdirs to the test suite
  3. White-spacing/formating changes

  4. The actual test_mkdirs function being moved from the top of the file to the bottom

    Ideally, this would only be changes (1) and (2). I'm okay with (3) slipping into this PR, especially since the codebase has no automatic formatting. However, (4) seems to be a change that is a resulting of first deleting that function and then rewriting it somewhere else. It would be good if the history wasn't like that.

    So concretely, I think we want to redo this PR with just (1), (2) and (3). Does that make sense?

create2000 commented 1 month ago

Thank you @patricoferris . I understand now what you mean. I will close this PR and here is the new PR created to keep everything clean. I hope the new PR meets the corrections.

Also, In recording my contribution on the Outreachy contribution page, I am asked to provide a URL of my contributions if it is merged or accepted. As this is still ongoing, can I still submit a URL in the hope that it will get merged before the contribution deadline? This is to keep with the advice of Outreachy to record our contributions early.