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

Code tidy up for you #1

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open

Conversation

timabell
Copy link

Watcha, cheers for yet another excellent piece of code and very helpful blog post.

I've done some tidying up and documenting of your module's code, and thought you might like it.

There shouldn't be any changes to the functionality in this set of changes.

More to follow!

Tim Abell

timabell added 30 commits March 22, 2012 16:36
Added sln file for convenience.
Removed irrelevant cywin path from ignore file.
Added nuget packages/ to ignore file.
Should be much clearer what calls to this are doing now. :-)
This should improve clarity of what the class is and what it's for.
For extra clarity. I had difficulty telling apart the framework from the organisation classes.
To show that they aren't using anything non-static.
More code doc for PageStructureBuilderModule class.
personal preference
Noticed a difference in the two code paths, need to figure out why they are different and document it.
@timabell
Copy link
Author

I've added a whole load more rework, hopefully it's more self-describing, and usable as a library now. It's now significantly different to your blog post so if you did pull it in, you may want to update it a bit so the names match. Sorry!

I understand it now, they are both actually PageData objects
I don't yet understand what the purpose of this feature is.
@timabell
Copy link
Author

There's probably not going to be much more now as I think I understand it enough to make use of it now. I'll let you be the judge of whether what I've done is an improvement or not :-)

Put the background at the end of the readme.
Added a summary line to the readme.
Move build dependencies above code usage.
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.

1 participant