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

Reduce number of IDE errors and wrong imports #913

Merged

Conversation

kornys
Copy link
Contributor

@kornys kornys commented Sep 6, 2023

No description provided.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Robocop found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
359 0 0 359 100

@kornys
Copy link
Contributor Author

kornys commented Sep 6, 2023

@bdattoma @jgarciao

.gitignore Show resolved Hide resolved
@bdattoma
Copy link
Contributor

bdattoma commented Sep 6, 2023

I personally don't like having every time the Library name in the keyword:

  • it reduces readability of the code
  • it makes writing code slower

Of course, if the majority of the team agrees with this choice, let's merge it

@Frawless
Copy link
Contributor

Frawless commented Sep 6, 2023

@bdattoma I am not sure if it reduces readability of the code especially if there are two sources from where the method could be called. Both Pycharm and VScode raised it as a warning which made us thinking why and what exactly is going on within the step. Unless there is a better solution (some config for robot lib?) I think this is a good solution

@bdattoma
Copy link
Contributor

bdattoma commented Sep 6, 2023

why and what exactly is going on within the step

I think there is nothing wrong going on with the test step itself. We set the library search order (i.e., Set Library Search Order SeleniumLibrary) in the suite setup so that Robot Framework can run the code. IDE are not able to capture the search order set in the test. I'm not aware of IDE configs about it

@kornys
Copy link
Contributor Author

kornys commented Sep 6, 2023

I also didn't find any config for robotframework-lsp which can respect library order

@kornys
Copy link
Contributor Author

kornys commented Sep 6, 2023

@bdattoma I just found setting in robotframework-lsp to disable this lint warning -> So I will remove SeleniumLibrary prefix from methods and leave just import fixes. And update doc what it shouldbe setup in robotgramework-lsp

@kornys kornys force-pushed the select-selenium-vs-jupyter branch from 163746e to eb3aa91 Compare September 7, 2023 07:51
@kornys
Copy link
Contributor Author

kornys commented Sep 7, 2023

@bdattoma @jgarciao jenkins was triggered with this change and no issue.

@kornys kornys force-pushed the select-selenium-vs-jupyter branch from eb3aa91 to 9907434 Compare September 7, 2023 08:34
@kornys kornys requested a review from bdattoma September 8, 2023 07:23
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@bdattoma bdattoma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgmt, thanks!

just found setting in robotframework-lsp to disable this lint warning

which setting of robotframework-lsp are you referring to?

@kornys
Copy link
Contributor Author

kornys commented Sep 8, 2023

lgmt, thanks!

just found setting in robotframework-lsp to disable this lint warning

which setting of robotframework-lsp are you referring to?

"robot.lint.keywordResolvesToMultipleKeywords": false

@bdattoma bdattoma added verified This PR has been tested with Jenkins enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) labels Sep 11, 2023
@bdattoma bdattoma merged commit 4d598e0 into red-hat-data-services:master Sep 11, 2023
ChughShilpa pushed a commit to ChughShilpa/ods-ci that referenced this pull request Nov 28, 2023
ChughShilpa pushed a commit to ChughShilpa/ods-ci that referenced this pull request Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants