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

Script to redeploy testlib.sml to all exercises on changes. #206

Merged
merged 11 commits into from
Jun 20, 2022
Merged

Script to redeploy testlib.sml to all exercises on changes. #206

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
156c591
Script to redeploy testlib.sml to all exercises on changes.
guygastineau Jun 11, 2022
627c2dc
Change file perms to 664 for sml-bin/redeploy-testlib.sml
guygastineau Jun 12, 2022
110d8d7
Make redeploy-testlib phone as well.
guygastineau Jun 12, 2022
721dd7b
Use qualified names.
guygastineau Jun 12, 2022
32774e5
Format redeploy-testlib.sml like teslib.sml
guygastineau Jun 12, 2022
f276a74
Include testlib sync policy in the README.
guygastineau Jun 14, 2022
d4c26df
Formatting, names, parentheses.
guygastineau Jun 14, 2022
48de3e8
Update README.md
guygastineau Jun 16, 2022
f75f658
Add spaces inside tuples.
guygastineau Jun 16, 2022
57b3885
Use scripts directory instead of sml-bin.
guygastineau Jun 20, 2022
38dff0e
Add a documenting comment to redeploy-testlib.sml.
guygastineau Jun 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@ test-%:
@cd ./exercises/practice/$(exercise) && cat test.sml | sed 's/$(exercise).sml/.meta\/example.sml/' | poly -q
@echo

redeploy-testlib:
poly --script sml-bin/redeploy-testlib.sml

.PHONY: test
Copy link
Contributor

Choose a reason for hiding this comment

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

redeploy-testlib should be .PHONY too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, cool. I never knew what .PHONY does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I read up on it. Yes, this makes perfect sense, and it is now done.

45 changes: 45 additions & 0 deletions sml-bin/redeploy-testlib.sml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
open OS.Path;
open OS.FileSys;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer not to use open. Makes some things easier. E.g. when I want to know what access does I first have to find out where it comes from (OS.Path or OS.FileSys, probably not difficult to guess but still easier if it is explicit). What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this was a "script", especially originally, I figured using open should be fine. I am not against using qualified names though. I will probably make more convenient names aliasing the structures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched everything to qualified names now.

Copy link
Contributor

Choose a reason for hiding this comment

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

The aliases are a good compromise :) .


fun printLn x = print (x ^ "\n")

fun fileExists x = access (x,[])

fun testLibName base slug =
joinDirFile
{
dir = (concat (base,slug))
, file = "testlib.sml"
}

fun dirList dirname = let
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make the formatting more consistent with the other sml-files (e.g. testlib.sml)? E.g. always indent by two spaces and for example

fun foo = 
  let
    val a = ...
  in
    do_something ...
  end; (* Note: there is a semicolon. *)

instead of

fun foo = let
    val a = ...
in
    do_something ...
end;

I have the feeling that this is better. But feel free to criticize this :) .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll make some changes. I need to figure out how to set sml-mode to use 2 spaces. It's indentation is worse than indentation in most emacs major modes. There really shouldn't be semicolons at the end of function definitions like this. Semicolons are for sequencing typically imperative actions. I had them in the first version of this script, but I think we should avoid them except where they are meaningful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally, I don't think there is an issue with let being on the opening line of the function, but I don't mind changing it for consistency.

I write a lot of haskell at work, so I am addicted to commas to the left in records and lists ;p

Copy link
Contributor

Choose a reason for hiding this comment

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

OK I am fine with omitting the semicolons. I was used to them since in the Book "ML for the working programmer" the author uses them wherever they are allowed (not just in the REPL or for imperative actions). But he actually states somewhere that they are optional there. It seems to be a matter of taste. I followed the author in my code, since I was used to something similar in javascript: In js "most of the time" the semicolons are not relevant, but not always, and therefore the general advice for js is to put them everywhere (I know javascript is probably not the most well designed language :D).

So since testlib already omits them we should omit them everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, with JS including the semicolons makes sure everything works as intended. With SML, including semicolons can change the meaning of a program 🤯

fun go d xs =
case readDir d of
SOME x => go d (x::xs)
| NONE => (closeDir d; xs);
in
go (openDir dirname) []
end

fun fileBytes path = let
val in' = BinIO.openIn path;
val bytes = BinIO.inputAll in';
in
BinIO.closeIn in'; bytes
end

fun writeFileBytes bytes path = let
val out = BinIO.openOut path;
in
BinIO.output (out, bytes);
BinIO.closeOut out
end

val _ = let
val concept = concat ((getDir ()), (fromUnixPath "exercise/concept"));
val practice = concat ((getDir ()), (fromUnixPath "exercises/practice"));

val testLib = fileBytes "lib/testlib.sml";
in
app (writeFileBytes testLib o testLibName practice) (dirList practice)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a very short docstring explaining what the script does might be a good idea (maybe at the top of the file). Of course the filename essentially says it but I would still prefer a short docstring. Let me know if you think this would not be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A comment basically restating the name of the script seems pretty redundant to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK I am fine with that, I was just a little bit worried if somebody else might wonder what exactly "redeploy" might mean.

Copy link
Contributor Author

@guygastineau guygastineau Jun 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, well, I often take for granted that English is my first language, aber meine Meinung nach sollen wir die Problemen sprachlicher Mehrdeutigkeit der Nicht-Muttersprachlichern beachten 🙃

So, maybe it is best for there to be some more direct comments in the script 😄

end