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

RsFlash is not tested #47

Closed
yegor256 opened this issue Mar 12, 2015 · 16 comments
Closed

RsFlash is not tested #47

yegor256 opened this issue Mar 12, 2015 · 16 comments

Comments

@yegor256
Copy link
Owner

RsFlash is not tested, let's create a unit test for it. Besides that, the class doesn't use the logging level provided anyhow. It simply ignores this parameter. Instead, it has to package the message and the logging level together in a single string and send them in Base64 format to the cookie. Without that we may have issues with unprintable characters inside the message.

@davvd davvd added the bug label Mar 12, 2015
@davvd
Copy link

davvd commented Mar 12, 2015

@yegor256 thanks, I added the "bug" tag

@davvd
Copy link

davvd commented Mar 13, 2015

@yegor256 thanks for the report, I topped your acc for 15 mins, payment ID 000-c0865f31

@davvd
Copy link

davvd commented Mar 15, 2015

@mgovorischev can you please help? Keep in mind this. If you have any technical questions, don't hesitate to ask right here. Task's budget is 30 mins (see this for explanation)

@mgovorischev
Copy link

What is the format of the string that contains log level and message and is then sent to the cookie? What is the delimiter?

@yegor256
Copy link
Owner Author

@mgovorischev it's up to you to decide. now we're using a simple string without any formatting, as you see. but we should package the message and the log level somehow. maybe

URLEncode(msg) + "/" + URLEncode(level)

in this case we won't need Base64

@davvd
Copy link

davvd commented Mar 26, 2015

@dmzaytsev it's yours now, please proceed keeping in mind our principles. Feel free to ask any technical questions right here in the ticket

30 mins is the budget of the task. This is exactly how much will be paid when the problem is solved (no matter how much time you will actually spend). See this for more information

dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 26, 2015
dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 26, 2015
dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 26, 2015
dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 26, 2015
@dmzaytsev
Copy link
Contributor

@yegor256 @davvd
I need more time to perform additional requirements.
#78 (comment)
May be better to create another task for this ?
What do you think ?

@yegor256
Copy link
Owner Author

@dmzaytsev you can try to use PDD (http://www.yegor256.com/2009/03/04/pdd.html). But we can't let these changes into the master, since they will break the entire "flash" functionality. It will basically stop working.

@dmzaytsev
Copy link
Contributor

@yegor256 @davvd
as I understand it, PDD is not suitable in this case.
Can we make a new task (TsFlash modification + unit test) and assign it to me?
I will complete both tasks(#47 and the new task) in the same pull request.

@yegor256
Copy link
Owner Author

@dmzaytsev we never mix tasks, we work with them individually, always. In this case, I would add a decrypting functionality to TsFlash and that's it. If you really want to avoid that, you should step back, remove the functionality you implemented, create a test that reproduces the connectivity between TsFlash and RsFlash, and add a @todo puzzle requesting the implementation of the desired functionality. In my opinion, that's more work (but that would the right/perfect way)

dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 29, 2015
dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 29, 2015
dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 30, 2015
dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 30, 2015
dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 30, 2015
dmzaytsev added a commit to dmzaytsev/takes that referenced this issue Mar 31, 2015
@dmzaytsev
Copy link
Contributor

@yegor256 the PR #78 merged
take a look at the changes
if you accept it close the task please

@yegor256
Copy link
Owner Author

yegor256 commented Apr 2, 2015

@dmzaytsev thanks!

@yegor256 yegor256 closed this as completed Apr 2, 2015
@davvd
Copy link

davvd commented Apr 3, 2015

@elenavolokhova please, review this task for compliance with our quality rules

@elenavolokhova
Copy link

@davvd This ticket looks good! :)

@davvd
Copy link

davvd commented Apr 4, 2015

@davvd This ticket looks good! :)

@elenavolokhova thanks for the QA review

@davvd
Copy link

davvd commented Apr 4, 2015

@dmzaytsev just added 10 mins to @elenavolokhova (for QA), payment ID is 54704490; I added 38 mins to your account, many thanks for working with the project! The completion time here was 54704500.; the bonus for fast delivery (age=2888); +38 added to your rating, current score is: +286

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

No branches or pull requests

5 participants