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

Reformatting of docs #532

Merged
merged 6 commits into from
Jan 11, 2016
Merged

Reformatting of docs #532

merged 6 commits into from
Jan 11, 2016

Conversation

kenbailey
Copy link
Contributor

Reformatting of docs with a Getting Started and Advanced topics section.
Attempted to incorporate all the existing /docs & /specs help files.
Removed previous /docs & /specs files to avoid confusion.

You can browse the new docs table of contents on my doc-updates branch

Ken Bailey added 3 commits December 23, 2015 22:54
@kenbailey
Copy link
Contributor Author

@paulcbetts, I'm looking for some feedback on this reformat of the Squirrel documentation. I'd be happy to make adjustments to the format and structure and wondered about moving the documentation to the wiki. Thoughts?

@hashhar I've added the source links you submitted in your PR (MSI Link, GitHub Install Link).

@hashhar
Copy link

hashhar commented Dec 30, 2015

Great. One thing I think the docs should be clear about is that:

  1. If I set up custom handlers for all Squirrel events, do the default handlers never get called?
  2. If I set up custom handlers for only one Squirrel event, do I have to handle the other events as well or will they be handled the default way?

@anaisbetts
Copy link
Contributor

Hey Ken, thanks for looking into this - the ordering of the docs were definitely something that can be improved. However, I think instead of coding it into the filename, if we put an index.md with a Table of Contents, that will end up better.

Also, is this just reorganization? Or has content been added / changed? Don't worry about it for now, but in the future try to do the "just reorganize" PR first, then do the "add stuff" PR, so it's easier to tell what's changed

@anaisbetts
Copy link
Contributor

If I set up custom handlers for all Squirrel events, do the default handlers never get called?

Correct, I tried to make this clear in the docs

If I set up custom handlers for only one Squirrel event, do I have to handle the other events as well or will they be handled the default way?

The former, if you opt-into Squirrel events you do everything yourself. For simple apps, install and update usually do the same thing, uninstall does the opposite, and obsolete usually does nothing

@hashhar
Copy link

hashhar commented Dec 30, 2015

Thanks for the clarification @paulcbetts. Would be helpful here.

@kenbailey
Copy link
Contributor Author

I think instead of coding it into the filename, if we put an index.md with a Table of Contents, that will end up better.

The intent was for the user to use the README.md as the starting point and then navigate via the MarkDown links in the documents. I'll try adding an index.md to the docs folder with a table of contents. A single table of contents for the advanced topics could simplify the navigation.

Also, is this just reorganization? Or has content been added / changed? Don't worry about it for now, but in the future try to do the "just reorganize" PR first, then do the "add stuff" PR, so it's easier to tell what's changed

Great feedback. The PR does include reorganized, new, and updated content. I can see where separate PR's would make it easier to review. I'll keep that in mind for the future. Let me work on an index.md and simplify the file naming. Then I can provide you some guidance regarding new vs. updated vs. reorganized to make review eaiser. Thanks.

Ken Bailey added 2 commits December 30, 2015 23:13
Removed numbering on file naming and separated into separate sub-directories for getting-started, using, and contributing.
@kenbailey
Copy link
Contributor Author

@paulcbetts, I believe the documentation is now ready for review. I think the single table of contents approach is working much better.

I believe all the relevant documentation from the previous /docs and /specs directories have been included in this reorganization. There are also a number of new files and an expanded getting started section.

@anaisbetts
Copy link
Contributor

@kenbailey Cool! I'll have a look soonish

@JVimes
Copy link

JVimes commented Jan 4, 2016

@kenbailey, In Step 1. Integrating it says, "Tip: In this example we simply put the code in the Program.cs file. For a production application, place the update code later in start-up process so as to avoid slowing down your program start." It looks like you're calling UpdateApp() on a new thread (Task.Run(async () =>...), so I think it won't lag the program startup. Maybe that tip is obsolete now?

@kenbailey
Copy link
Contributor Author

@JVimes, yes it's would be applied to the task pool so I would think the overhead would be minimal. I can see some value in delaying the start so the update process isn't competing for resources or network bandwidth while the app is loading. How about the following:

Task.Run(async () =>
{
    await Task.Delay(1000 * 5); // wait 5 seconds to start update process
    using (var mgr = new UpdateManager("C:\\Projects\\MyApp\\Releases"))
    {
        await mgr.UpdateApp();
    }
});

Anyone see any other concerns or reasons to start the update process inside the application context (Application.Run()) rather than simply calling in Main()?

@JVimes
Copy link

JVimes commented Jan 4, 2016

@kenbailey, I see what you mean, but it seems like the slow-down would be unnoticeable in most cases. Should we be including that in the "Getting Started" section? Seems like a better candidate for an "Advanced Considerations" section (probably there's a better title than that).

@anaisbetts
Copy link
Contributor

@kenbailey Yo, where'd this great logo come from? Love it!

@anaisbetts anaisbetts merged commit 0fe19da into Squirrel:master Jan 11, 2016
anaisbetts added a commit that referenced this pull request Jan 11, 2016
@anaisbetts
Copy link
Contributor

This is excellent work, thank you so much @kenbailey!

@kenbailey
Copy link
Contributor Author

@paulcbetts, I wasn't terribly excited by the Logo for the general Squirrel group so decided to take a crack at a new one. The logo is a composite of a couple of public domain svg images from thenounproject (Squirrel, Package), with some modifications. When I get a chance I'll do a few more png sizes and possible an icon file.

@anaisbetts
Copy link
Contributor

@kenbailey The SVG is good enough, we can always make PNGs at most reasonable sizes based on that

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.

4 participants