-
Notifications
You must be signed in to change notification settings - Fork 94
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
Various updates #13
Various updates #13
Conversation
…ate key), and sorted words list.
…upported for Russian. Upgraded to .NET 4.6.1.
* As hilarious as the "http/https:" as code tricks are, they generate build warnings.
* As hilarious as the "http/https:" as code tricks are, they generate build warnings.
* Most notably, shape words (IE "triangles", "ovals"), "babysmash", and "scott"
* Most notably, shape words (IE "triangles", "ovals"), "babysmash", and "scott"
* Similar amount of code, but does seem like a more natural/extensible approach.
* Similar amount of code, but does seem like a more natural/extensible approach.
* Ideally a build process would generate online-available zips or Win-10-capable installers and keep them out of source control, but for now, we may as well periodically update this.
* Ideally a build process would generate online-available zips or Win-10-capable installers and keep them out of source control, but for now, we may as well periodically update this.
Cool! This will take me a while to look through... |
Any news on this, I would be especially interested in the localization feature and willing to add german. |
Eek! I need some help, I'm travelling. Someone want to be a merger/viewer? @razzeee @ericlaw1979 |
Well maybe we can get @DavidRieman to split these additions into more PRs? It's a pretty big PR with alot of commits/code changed. |
The GitHub "Commits" tab produces a very nice way to review one commit at a time, since my commit workflow is pretty granular in that way. I would be happy to address code review comments made at a per-commit level. (I would prefer to avoid the overhead of making a bunch of new branches and cherry-picking commits to push and make new pull requests, if reviewing per commit is a suitable workflow, so let me know.) |
I understand. I'm aware of the github feature here I just know and like the practice to separate prs by topic. As that might lead to the trivial stuff getting merged faster and not getting blocked by stuff that needs discussion. |
Do you just want commit, @DavidRieman or should I just merge this? |
Not sure 33812e5 is a good idea generally. You can attach binaries to 'releases' on GitHub without adding them to the repo. |
BabySmash.csproj
Outdated
@@ -10,14 +10,14 @@ | |||
<AppDesignerFolder>Properties</AppDesignerFolder> | |||
<RootNamespace>BabySmash</RootNamespace> | |||
<AssemblyName>BabySmash</AssemblyName> | |||
<TargetFrameworkVersion>v3.5</TargetFrameworkVersion> | |||
<TargetFrameworkVersion>v4.6.1</TargetFrameworkVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason to increase the minimum supported framework? The higher this number is, the fewer people can run this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it was intended. Those were merged in from @katu07 language support. I've pulled it back to 3.5 now.
* Updates Newtonsoft.Json lib to latest 3.5-compatible version. * Removes unused, otherwise incompatible System.Xaml reference
* Updates Newtonsoft.Json lib to latest 3.5-compatible version. * Removes unused, otherwise incompatible System.Xaml reference
* Needs further discussion for a final solution.
* Needs further discussion for a final solution.
I believe I have addressed all the PR feedback. Let me know if I missed something.
|
@shanselman I'm not certain what you were asking. If it is "would you like to commit PR feedback..." then the yes. |
@DavidRieman it'd be better to squash/fixup the add/remove commits so the zip doesn't end up in the repo history. You can do this with an interactive rebase, then force push. |
So should I merge now or wait @drewnoakes ? |
Up to you. If you accept the PR by squashing all commits (via the dropdown) then the binaries won't end up in the history. Instead you'll end up with one large commit that loses the granular messages @DavidRieman put against each commit. If it were my project I'd wait for the two commits to be squashed (or do it myself). I didn't see how large the binaries are, but right now your repo is nice and small. |
I'd be happy to look into squashing on my end; I'm long overdue for learning how to do that anyway. It may be another day or so before I can dive into that though. |
Look into interactive rebase. For example:
This launches an editor with a script that you can edit to modify how commits are rebased. You can reorder, drop commits, squash, and so forth. It's very powerful. Save and exit the editor to start the process. You might hit conflicts as it plays the script, which you'd have to resolve. Once done you can push your branch. Because you'll have edited the history, you'll need to force push. Hope that helps. |
Ok, rebase is still a little scary to me but I got the job done with: |
So do I merge now? |
My end is All Clear for merge. |
Default dictionary updated, hotkey command addressed, katu07's localization support, and cleanup.
Works on Win 10 but the taskbar likes to show.