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

add variable capture example #3

Merged
merged 2 commits into from
Feb 21, 2019
Merged

Conversation

oxinabox
Copy link
Contributor

This is a basically what is currently in
oxinabox/MagneticReadHead.jl#14

I think it is best to share it now as it is before it gets more complex

@jrevels
Copy link
Owner

jrevels commented Feb 21, 2019

Awesome, thanks!

Can we get rid of the passes directory? I'd rather just keep a flatter directory structure for now; the top-level directories for this repo can just be the examples themselves, and if it gets too big, we can sort it out later.

Hmmm. What's the best way to set up CI testing here? We could just have a test script that loops through each project folder in the repo, activates/builds the project (whatever the Pkg3 lingo is for this), and runs a runtests.jl file in each project. Maybe in that case it does make sense to actually make this repo a bona fide Julia package (containing many subprojects).

@oxinabox
Copy link
Contributor Author

What do you think of the order of functions in this example.
It follows my habit of putting things in order such that they are defined before the line where they are called, when possible.

But this means it goes from lowest level helper down to highest level function that does the thing.
Which might make for harder to read tuitorial.

Can we get rid of the passes directory? I

Done

Hmmm. What's the best way to set up CI testing here? We could just have a test script that loops through each project folder in the repo, activates/builds the project (whatever the Pkg3 lingo is for this), and runs a runtests.jl file in each projec

Sounds sensible.
@MikeInnes Flux model zoo should do this too. but seems not to?

Maybe in that case it does make sense to actually make this repo a bona fide Julia package (containing many subprojects).

I don't think so.
I've recently seen what happens when examples become registered as packages and it is not nice when people start depending on them.

@jrevels
Copy link
Owner

jrevels commented Feb 21, 2019

I've recently seen what happens when examples become registered as packages and it is not nice when people start depending on them.

It doesn't need to be registered, I just mean make it a package so we can use the default Julia package test config without too much thinking

@jrevels
Copy link
Owner

jrevels commented Feb 21, 2019

But this means it goes from lowest level helper down to highest level function that does the thing. Which might make for harder to read tuitorial.

I think going in your current order is sensible 🙂regardless, it's not a super long file so the scrolling burden isn't that bad

@oxinabox
Copy link
Contributor Author

It doesn't need to be registered, I just mean make it a package so we can use the default Julia package test config without too much thinking

You mean we should have a .travis.yml file.
That vaguely looks like a normal julia one.

@oxinabox
Copy link
Contributor Author

Anything blocking?

@jrevels jrevels merged commit eaf25a2 into jrevels:master Feb 21, 2019
@oxinabox oxinabox deleted the ox/varnames branch February 21, 2019 21:58
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.

2 participants