Optional return type of int#717
Optional return type of int#717GLMeece wants to merge 1 commit intorobotframework:masterfrom GLMeece:patch-3
int#717Conversation
Changed `get_matching_xpath_count` such that returning an integer is an option instead of only a string. See also https://github.com/robotframework/Selenium2Library/issues/715
| # Public, xpath | ||
|
|
||
| def get_matching_xpath_count(self, xpath): | ||
| def get_matching_xpath_count(self, xpath, return=str): |
There was a problem hiding this comment.
Perhaps return_str=True would be better argument definition.
- It would not be confused with Python return statement.
- Then you could do something like this (in lines 625 - 628):
return str(count) if return_str else count
|
First of all, it would be a good idea to test the code at least manually before submitting a PR. I also agree with @aaltat that specifying return type freely wouldn't give much benefits when there's only two meaningful values. Having something like With Boolean values we should just think how to handle string |
|
I didn't know that Robot Framework has such convince methods. The programmer me says that they are not needed but tester me like's it. I don't remember do we have booleans somewhere in the keywords arguments, need to check it out. |
|
There are three keywords, the Add Location Strategy and two alert keywords, which directly had Perhaps supporting Robot Framework truth evaluation is a separate issue and for this PR using the Python truth evaluation is best way to go. I agree that Robot Framework truth evaluation is better and we should take it in use also in S2L. |
|
Yes, consistent handling of Boolean arguments is a separate issue. Here's the original RF issue: robotframework/robotframework#2038 |
|
Mea culpa - I shouldn't have rushed this (I had family over, but that's no excuse). Yes, I'll work on this today, but it may be 10+ hours from now before I have a chance. |
|
Take your time. It strongly looks like that we are not going to make a Xmas release. |
|
@GLMeece Would you have time to make the improvements? |
|
Hey, @aaltat - I have been slammed at work along with family issues (my parents are ill). At best, I might be able to take a look this weekend. |
|
I understand, hope your parents get soon better. Pekka would like to start the new architecture planning/coding soon but first we would like to get some PR to the current master code line. And this PR is one of those PR. There is not rush and waiting to the weekend is totally fine. I was more interested on a status update. And before you send the new commit, could you rebase with the current master branch. It contains CI improvements and now test are run automatically with Python 2&3 and selenium 2&3. But the Python 3 tests fails, because the Python 3 support is not yet completed. |
|
Closed to favor of #735 |
Changed
get_matching_xpath_countsuch that returning an integer is an option instead of only a string.See also https://github.com/robotframework/Selenium2Library/issues/715