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

support xml:lang in release notes #70

Closed
wants to merge 2 commits into from

Conversation

HuangKBAaron
Copy link

Before the fix, winsparkle only picks the first item, not picking the right language attribute set by win_sparkle_set_lang function.

…le only picks the first item, not picking the right language attribute set by win_sparkle_set_lang function.
@vslavik
Copy link
Owner

vslavik commented Sep 13, 2015

Thanks!

Can you please update the commit message to be correct (this doesn't "fix" anything, it implements new functionality), mention that it closes #35 and make it so that it follows the usual formatting of git commit logs? (Please see https://github.com/agis-/git-style-guide — you can use git push -f to update this PR.)

@vslavik
Copy link
Owner

vslavik commented Sep 13, 2015

In addition to the comments above, this also breaks handling of appcasts without language-tagged release notes where the user code did explicitly set the language.

@HuangKBAaron
Copy link
Author

ic. It is a quick solution in my case, I will spend sometime to make a sounded solution later.

@GitCop
Copy link

GitCop commented Sep 14, 2015

Thanks for contributing! Unfortunately there were the following style issues with your Pull Request:

  • Commit: eb7d349
    • Your subject line is longer than 50 characters

Please see https://github.com/agis-/git-style-guide (you can use git push -f to update this PR)


This message was auto-generated by https://gitcop.com

@vslavik
Copy link
Owner

vslavik commented Sep 14, 2015

I will spend sometime to make a sounded solution later.

What about doing it sooner than what I suspect means "never"? You already spent time doing the PR (appreciated!), I spent time reviewing it (instead of just closing it as broken), WinSparkle could use some quality giving back...

@GitCop
Copy link

GitCop commented Sep 15, 2015

Thanks for contributing! Unfortunately there were the following style issues with your Pull Request:

  • Commit: eb7d349
    • Your subject line is longer than 50 characters

Please see https://github.com/agis-/git-style-guide (you can use git push -f to update this PR)


This message was auto-generated by https://gitcop.com

@HuangKBAaron
Copy link
Author

:) i spent 30 minutes to learn lambda, here is the update.

@vslavik
Copy link
Owner

vslavik commented Sep 15, 2015

:) i spent 30 minutes to learn lambda, here is the update.

Thanks, but unfortunately you didn't address either of the major issues: flawed language detection and the fact that it breaks feeds that don't use xml:lang. I simply can't merge this like this.

Also, please don't ignore the request for proper git commit messages. It's basic code hygiene and I have hard time believing you don't see how uninformative a commit message like "change approach" is.

@vslavik vslavik changed the title fixed the multi-localized release notes bug. support xml:lang in release notes Oct 3, 2015
@vslavik
Copy link
Owner

vslavik commented Sep 3, 2016

Closing this PR because the author abandoned it in half-finished state.

@vslavik vslavik closed this Sep 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants