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

Update stringex to handle special chars in API calls #847

Closed
wants to merge 1 commit into
base: 0-70-stable
from

Conversation

Projects
None yet
5 participants
@sambauers

sambauers commented Dec 6, 2011

Stringex has been updated recently to fix an error in YAML parsing which pops up when using non-ascii characters in API requests.

Updating to stringex 1.3.0 seems to fix this.

I understand that stringex is not used in the current master branch, but this would still be good for the current stable branch.

@joneslee85 joneslee85 closed this Dec 6, 2011

@sambauers

This comment has been minimized.

Show comment
Hide comment
@sambauers

sambauers Dec 6, 2011

Wow, that was quick. Thanks.

sambauers commented Dec 6, 2011

Wow, that was quick. Thanks.

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Dec 6, 2011

Member

Ah, we stopped using Stringex on the master branch because of this YAML parsing error! I think parameterize does exactly what we need in Spree and so I'm going to leave it like that on master.

Member

radar commented Dec 6, 2011

Ah, we stopped using Stringex on the master branch because of this YAML parsing error! I think parameterize does exactly what we need in Spree and so I'm going to leave it like that on master.

@romul

This comment has been minimized.

Show comment
Hide comment
@romul

romul Dec 7, 2011

Member

I think parameterize does exactly what we need in Spree and so I'm going to leave it like that on master.

@radar: You make wrong decision. The worst thing that you did it in the master without any approval.
parameterize works fine only for english and not so bad for something latin languages, but for many other languages by default people will get blank string:

# without stringex
irb(main):001:0> "做之前需要思考的东西".parameterize
=> ""
irb(main):002:0> "надо думать прежде чем делать".parameterize
=> ""
# with stringex
irb(main):003:0> "做之前需要思考的东西".to_url
=> "zuo-zhi-qian-xu-yao-si-kao-de-dong-xi"
irb(main):004:0> "надо думать прежде чем делать".to_url
=> "nado-dumat'-priezhdie-chiem-dielat'"
Member

romul commented Dec 7, 2011

I think parameterize does exactly what we need in Spree and so I'm going to leave it like that on master.

@radar: You make wrong decision. The worst thing that you did it in the master without any approval.
parameterize works fine only for english and not so bad for something latin languages, but for many other languages by default people will get blank string:

# without stringex
irb(main):001:0> "做之前需要思考的东西".parameterize
=> ""
irb(main):002:0> "надо думать прежде чем делать".parameterize
=> ""
# with stringex
irb(main):003:0> "做之前需要思考的东西".to_url
=> "zuo-zhi-qian-xu-yao-si-kao-de-dong-xi"
irb(main):004:0> "надо думать прежде чем делать".to_url
=> "nado-dumat'-priezhdie-chiem-dielat'"

@romul romul reopened this Dec 7, 2011

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Dec 7, 2011

Member

@romul: Your passive aggressiveness has no place here. There are plenty of ways to tell people that they incorrect without offending them. Please refrain from doing this in the future.

So I was wrong. It happens. I will fix this up today now that stringex isn't broken.

Member

radar commented Dec 7, 2011

@romul: Your passive aggressiveness has no place here. There are plenty of ways to tell people that they incorrect without offending them. Please refrain from doing this in the future.

So I was wrong. It happens. I will fix this up today now that stringex isn't broken.

@schof

This comment has been minimized.

Show comment
Hide comment
@schof

schof Dec 7, 2011

Member

@romul: @radar was simply trying to fix problem with Ruby 1.9.x and failing tests. No special permission is needed for this. Sounds like he overlooked something so thanks for pointing that out.

Member

schof commented Dec 7, 2011

@romul: @radar was simply trying to fix problem with Ruby 1.9.x and failing tests. No special permission is needed for this. Sounds like he overlooked something so thanks for pointing that out.

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Dec 8, 2011

Member

Fixed.

Member

radar commented Dec 8, 2011

Fixed.

@radar radar closed this Dec 8, 2011

@romul

This comment has been minimized.

Show comment
Hide comment
@romul

romul Dec 8, 2011

Member

@radar: Here is no aggressiveness. I truly believe that you and nobody else don't have the right to drop any Spree dependency without discussion with other members of Spree Core Team. I think, when you taking such individual decisions you show disrespect to all the Spree contributors. The fact that this has led to the error it's a trifle compared with the fact that it was not thought out collectively. Please don't forget in the future, that this is not your personal repository, i.e. observe the existing rules when work with master branch if provoking aggression isn't your goal!

Member

romul commented Dec 8, 2011

@radar: Here is no aggressiveness. I truly believe that you and nobody else don't have the right to drop any Spree dependency without discussion with other members of Spree Core Team. I think, when you taking such individual decisions you show disrespect to all the Spree contributors. The fact that this has led to the error it's a trifle compared with the fact that it was not thought out collectively. Please don't forget in the future, that this is not your personal repository, i.e. observe the existing rules when work with master branch if provoking aggression isn't your goal!

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Dec 8, 2011

Member

Insulting me in your OP in not one but THREE separate languages shows definite pre-meditated aggression. To suggest that I need approval for any change I make to this repository is ludicrous. The timezone difference alone does not cater to this.

Your passive aggressiveness is off-putting and unhelpful.

I realize that this is not my personal repository but that doesn't mean that I am completely unable to use my best judgement at a time to make a decision. If the core team wishes, they can review each commit and comment on it if necessary, as people like joneslee85 have been doing.

If there is something wrong, then I believe the Core team will bring it up in a respectful manner.

Like I said before, I made a mistake and I apologized for it. I spent the time yesterday morning working on fixing it, as well as writing regression tests for it.

But you did nothing other than complain about the mistake and insulted me personally by implying that I didn't think at all before making this change. I took great offense to not just your message, but the way it was phrased. This is not an isolated incidence, either.

Please be less harsh in the future. For example, you could have commented on the initial commit that removed stringex with examples that didn't work. In the case of that commit you commented on with "WTF?", show a way that it could be written better. If you want to earn other people's respect, this is how you're going to do it, not by proving that you're smarter than them.

Member

radar commented Dec 8, 2011

Insulting me in your OP in not one but THREE separate languages shows definite pre-meditated aggression. To suggest that I need approval for any change I make to this repository is ludicrous. The timezone difference alone does not cater to this.

Your passive aggressiveness is off-putting and unhelpful.

I realize that this is not my personal repository but that doesn't mean that I am completely unable to use my best judgement at a time to make a decision. If the core team wishes, they can review each commit and comment on it if necessary, as people like joneslee85 have been doing.

If there is something wrong, then I believe the Core team will bring it up in a respectful manner.

Like I said before, I made a mistake and I apologized for it. I spent the time yesterday morning working on fixing it, as well as writing regression tests for it.

But you did nothing other than complain about the mistake and insulted me personally by implying that I didn't think at all before making this change. I took great offense to not just your message, but the way it was phrased. This is not an isolated incidence, either.

Please be less harsh in the future. For example, you could have commented on the initial commit that removed stringex with examples that didn't work. In the case of that commit you commented on with "WTF?", show a way that it could be written better. If you want to earn other people's respect, this is how you're going to do it, not by proving that you're smarter than them.

@joneslee85

This comment has been minimized.

Show comment
Hide comment
@joneslee85

joneslee85 Dec 8, 2011

Member

@romul: I am trying to keep quiet for a while as I do not want to raise the flame up that could burn down the whole house but now I have to speak out. I am with @radar. I myself felt offended few times before with your agressive attitude. No matter how good you are, we are all human and we do make mistakes and you are no exception.

Something like "you are wrong" or "WTF" only has detrimental effect on building a healthy community. What would contributor perceive your message as? If they don't know you before, they would take it as the project is running by a bunch of tyrants. The core member therefore should not only be competent in technical understanding but also in communication, they, the keys of a project, an image to represent a community should be an image for the whole community to look up to. I beg you to please think before you say as the consequences are irrreversible.

Member

joneslee85 commented Dec 8, 2011

@romul: I am trying to keep quiet for a while as I do not want to raise the flame up that could burn down the whole house but now I have to speak out. I am with @radar. I myself felt offended few times before with your agressive attitude. No matter how good you are, we are all human and we do make mistakes and you are no exception.

Something like "you are wrong" or "WTF" only has detrimental effect on building a healthy community. What would contributor perceive your message as? If they don't know you before, they would take it as the project is running by a bunch of tyrants. The core member therefore should not only be competent in technical understanding but also in communication, they, the keys of a project, an image to represent a community should be an image for the whole community to look up to. I beg you to please think before you say as the consequences are irrreversible.

@romul

This comment has been minimized.

Show comment
Hide comment
@romul

romul Dec 9, 2011

Member

To suggest that I need approval for any change I make to this repository is ludicrous.

Seems, you read inattentively. Any significant change should be approved. Changing dependencies is certainly significant change.

If the core team wishes, they can review each commit and comment on it if necessary, as people like joneslee85 have been doing.

Make branches for non-trivial changes, stop spam to master(pushing untested commits and dozens minor commits instead of one holistic), and your commits will be reviewed as well. But commits should be reviewed before they come to master!!! And you totally close this opportunity and this is worst.
Currently, your commits offend me far more than any words. You abuse of your rights, making lots of small insignificant commits to the master and commits, which you to lazy to verify on localhost before applying to the master (so you apply buggy commit and fix it instead of fix it before applying).

I am with radar.

It is not surprising. Indeed, the lack of thought about the potential side effects before pushing commit inherent to you, no less than to Ryan. Even the idea that we should think about the consequences of commit before pushing it to the master perceived you as an insult. Although it's evident axiom for each commiter.

we do make mistakes and you are no exception.

Agreed. I don't blame anyone for the fact of mistakes, I only speak against carelessness, sloppiness and arbitrariness when it comes to master branch.

Something like "you are wrong" or "WTF" only has detrimental effect on building a healthy community. What would contributor perceive your message as?

Firstly, don't distort reality, as if I react so to any commit, such reaction worthy of only the most worst changes that do not have to get into the master.
Secondly, if I have enough spare time, I will prefer to correct your error with new commit than wasting time on writing a detailed comment. And if the little free time, I'm just pointing out the existence of the problem, this is more than enough.
And yes, sometimes "WTF?" is the best of possible descriptions. This means that the change is absolutely unclear and is likely to contain a logical error, which can a long time doesn't manifest itself. And there is nothing bad in such descripton... The only valid measurement of code quality is WTFs/minute ;-)

Member

romul commented Dec 9, 2011

To suggest that I need approval for any change I make to this repository is ludicrous.

Seems, you read inattentively. Any significant change should be approved. Changing dependencies is certainly significant change.

If the core team wishes, they can review each commit and comment on it if necessary, as people like joneslee85 have been doing.

Make branches for non-trivial changes, stop spam to master(pushing untested commits and dozens minor commits instead of one holistic), and your commits will be reviewed as well. But commits should be reviewed before they come to master!!! And you totally close this opportunity and this is worst.
Currently, your commits offend me far more than any words. You abuse of your rights, making lots of small insignificant commits to the master and commits, which you to lazy to verify on localhost before applying to the master (so you apply buggy commit and fix it instead of fix it before applying).

I am with radar.

It is not surprising. Indeed, the lack of thought about the potential side effects before pushing commit inherent to you, no less than to Ryan. Even the idea that we should think about the consequences of commit before pushing it to the master perceived you as an insult. Although it's evident axiom for each commiter.

we do make mistakes and you are no exception.

Agreed. I don't blame anyone for the fact of mistakes, I only speak against carelessness, sloppiness and arbitrariness when it comes to master branch.

Something like "you are wrong" or "WTF" only has detrimental effect on building a healthy community. What would contributor perceive your message as?

Firstly, don't distort reality, as if I react so to any commit, such reaction worthy of only the most worst changes that do not have to get into the master.
Secondly, if I have enough spare time, I will prefer to correct your error with new commit than wasting time on writing a detailed comment. And if the little free time, I'm just pointing out the existence of the problem, this is more than enough.
And yes, sometimes "WTF?" is the best of possible descriptions. This means that the change is absolutely unclear and is likely to contain a logical error, which can a long time doesn't manifest itself. And there is nothing bad in such descripton... The only valid measurement of code quality is WTFs/minute ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment