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

Adequately test all samples #32

Open
12 tasks
avescodes opened this issue Jul 14, 2013 · 5 comments
Open
12 tasks

Adequately test all samples #32

avescodes opened this issue Jul 14, 2013 · 5 comments
Labels

Comments

@avescodes
Copy link

Let's face the facts, our test coverage in samples is abysmal.

We need to evaluate and add tests for any samples where it makes sense.

@clojens
Copy link

clojens commented Aug 30, 2013

Parked the comments in my repo for now, I'll have a go at regenerating some of the samples. I'll start with cors.

@clojens
Copy link

clojens commented Sep 6, 2013

Ok I've completely regenerated the cors sample, it works flawlessly if executed in the correct order, and as long as the connection isn't suddenly terminated e.g. by closing the listening/sending socket which, obviously, will still dump the full exception stack to stdout/console. This is something which, if I recall correctly, was mentioned earlier as something to try and see if you guys could make some of those cases a bit more failt tolerant like the rest.

I did some work on the README, not too much, although I did take a little bit of creative freedom to add in a little extra on the usage description, and I was wondering what the stance/feeling on it is. The only thing that would be really new in the document would be the second Usage topic that explains/shows a little Light Table. It is not so much that I'm very opinionated on much things, editors included, although I find LT pretty sweet myself, I really feel in places these pictures can say more than a thousand words and gives instant impression on what a user would expect to see (even when not using LT of course, a lot will match). Some things you might even spend a whole paragraph (or two) trying to describe (such as expected behavior/output/messages printed) where this, on occasion, brings across the message stronger. Also its a lot nicer to take screenshots, when having everything stacked in 1 window.

Also since I didn't get a call yet on where and under which branch this should be posted, until I can do a pull request I'll leave it stashed here: https://github.com/clojens/app-tutorial/tree/refactor/media/new-samples/cors

@avescodes
Copy link
Author

Cool, I'm a little busy with some other Cookbook and Pedestal stuff, so I'll have to take a look later next week.

@clojens
Copy link

clojens commented Sep 13, 2013

Sure no problem. I'd like to run them by myself one more time as well.

Also, last few days I've been pondering some problems/challenges on the UI front. As a small up-step to some more significant infrastructure perhaps, I've taken the liberty to add CSS server-side templates to the server-templates sample. Plus I've added a semantic grid in garden DSL, I thought people might appreciate it nicely bundled over there, and I've tried to outline a bit more what the different options are you could take (added among others a plumbing graph, rather naive/simple implementation at the moment). Temporary repository https://github.com/clojens/app-tutorial/tree/refactor/media/new-samples/template-server/src/template_server

As with all I post, please mind that these are not for publication just yet and won't vouch for anything until I make the final pull requests at any official repo's (that's for people who might pick this topic/links up later). I did however try everything out at my system, works flawlessly. I will probably clean up the CSS sometime soon and write some tests as well, then play with some more ideas and probably beautify the samples a bit more (in the Relevance/Pedestal corporate house-style found on the site). That is since you are occupied a bit as well, I haven't settled on many things and we're not in any rush so...

@avescodes
Copy link
Author

Yeah, I like what you're doing on updating template server to include a few more options! Really cool stuff to see all of those sample. One note: Do you mind using marginalia-style Markdown formatting for comments? We're trying to make things consistent across the projects.

Also, we just merged an update of chat samples, FYI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants