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.

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

Turn exercise 009 into practice #2177 #2229

Merged
merged 15 commits into from
Mar 26, 2024
Merged

Turn exercise 009 into practice #2177 #2229

merged 15 commits into from
Mar 26, 2024

Conversation

Kxrishx03
Copy link
Contributor

Please review & suggest if any corrections needed.

@Kxrishx03 Kxrishx03 marked this pull request as ready for review March 16, 2024 07:09
@Kxrishx03 Kxrishx03 changed the title Practice009 -issue #2177 Turn exercise 009 into practice #2177 Mar 16, 2024
@divyankachaudhari
Copy link
Contributor

Please include a newline at the end of all the files.

@Kxrishx03
Copy link
Contributor Author

Please review now . @divyankachaudhari

@divyankachaudhari
Copy link
Contributor

divyankachaudhari commented Mar 19, 2024

@Kxrishx03 I'm just an outreachy applicant as well 😅 I was just trying to help you with the experience I got with my PRs.

@@ -0,0 +1,2 @@
(include_subdirs no)
(test (name run) (libraries ounit2 ex))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add newline at end of the file

Copy link
Collaborator

Choose a reason for hiding this comment

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

One more here

@@ -0,0 +1 @@
(lang dune 3.7)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add newline at end of the file

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, newline here too

if a = b then aux (a :: current) acc t
else aux [] ((a :: current) :: acc) t in
List.rev (aux [] [] list);;

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add newline at end of the file

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, here it's delete empty line and finish with newline

Copy link
Contributor

Choose a reason for hiding this comment

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

You've still modified the 001 folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do i remove that @divyankachaudhari ? I don't have much experience with github actually.

Copy link
Contributor

Choose a reason for hiding this comment

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

Revert the change you've made. Here in practice/001/answer/test/run.ml, you've accidentally added space in a new line, so just remove that. A way to prevent such accidental changes is when you use git add only specify git add <folder you're modifying> so you don't add any accidental changes to your commit :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Kxrishx03: Just do something like git checkout main -- practice/001/answer/run.ml and then git commit -a -m 'Do not touch irrelevant file'

Keep in mind that the added value of this internship is learning functional programming and ocaml, git is more a prerequisite ❤️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cuihtlauac Thanks for the reviews. I will make the requested changes right away!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cuihtlauac I've made changes as you suggested. Please review! Thank you so much!!!

@divyankachaudhari
Copy link
Contributor

@Kxrishx03 make sure dune build --root . and dune test --root . answer work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Kxrishx03: Just do something like git checkout main -- practice/001/answer/run.ml and then git commit -a -m 'Do not touch irrelevant file'

Keep in mind that the added value of this internship is learning functional programming and ocaml, git is more a prerequisite ❤️

@cuihtlauac cuihtlauac linked an issue Mar 19, 2024 that may be closed by this pull request
if a = b then aux (a :: current) acc t
else aux [] ((a :: current) :: acc) t in
List.rev (aux [] [] list);;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, here it's delete empty line and finish with newline

@@ -0,0 +1 @@
(lang dune 3.7)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, newline here too

@@ -0,0 +1,2 @@
(include_subdirs no)
(test (name run) (libraries ounit2 ex))
Copy link
Collaborator

Choose a reason for hiding this comment

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

One more here

Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

Take a look at the end of your files in your folder with hexdump or something similar. Compare with files from other folders. You have added empty lines, not just a mere newline character at the end of the text

@Kxrishx03
Copy link
Contributor Author

@cuihtlauac I've removed all the extra empty line & added new line.

@cuihtlauac cuihtlauac added the outreachy Outreachy contributions and blog posts label Mar 25, 2024
Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

Thanks @Kxrishx03

@cuihtlauac cuihtlauac merged commit d79c4b9 into ocaml:main Mar 26, 2024
1 check passed
@Kxrishx03
Copy link
Contributor Author

@cuihtlauac Hi! Thanks for guiding me through the task. I would like to keep contributing. Please assign me some tasks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
outreachy Outreachy contributions and blog posts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Turn exercise 009 into practice
3 participants