FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add excluded and mode attributes to Parameter by ivan-neverov-al · Pull Request #591 · allure-framework/allure-ruby · GitHub

/ allure-ruby Public

Add excluded and mode attributes to Parameter - #591

Merged
andrcuns merged 14 commits into
allure-framework:masterfrom
ivan-neverov-al:add_excluded_and_mode
May 20, 2025
ConversationCommits (14)ChecksFiles changed
Merged

Add excluded and mode attributes to Parameter#591
andrcuns merged 14 commits into
allure-framework:masterfrom
ivan-neverov-al:add_excluded_and_mode

Conversation

ivan-neverov-al commented May 17, 2025

Copy link
Copy Markdown
Contributor

Adding excluded and mode options for Parameter class for Ruby binding

ivan-neverov-al requested a review from andrcuns as a code owner May 17, 2025 03:08

CLAassistant commented May 17, 2025 •
edited
Loading

Copy link
Copy Markdown


All committers have signed the CLA.

andrcuns reviewed May 18, 2025
View reviewed changes

andrcuns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@ivan-neverov-al Thanks, changes look good to me, few minor questions from me.

Comment thread allure-ruby-commons/lib/allure_ruby_commons/model/parameter.rb
Comment thread allure-ruby-commons/lib/allure_ruby_commons/model/parameter.rb Outdated
ivan-neverov-al requested a review from andrcuns May 19, 2025 19:26
andrcuns added the enhancement Improvement or request label May 19, 2025

ivan-neverov-al commented May 19, 2025

Copy link
Copy Markdown
Contributor Author

@andrcuns missed one more rubocop fix

andrcuns requested changes May 19, 2025
View reviewed changes

andrcuns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's much better now, but I think it's better to have explicit constants for each mode as it is more explicit and avoids using plain strings in code, better for future maintainability

Comment thread allure-ruby-commons/lib/allure_ruby_commons/model/parameter.rb Outdated
Comment thread allure-ruby-commons/lib/allure_ruby_commons/model/parameter.rb Outdated
Comment thread allure-ruby-commons/lib/allure_ruby_commons/model/parameter.rb Outdated
Comment thread allure-ruby-commons/spec/unit/allure_spec.rb Outdated
Comment thread allure-ruby-commons/spec/unit/allure_spec.rb Outdated
Comment thread allure-ruby-commons/lib/allure-ruby-commons.rb Outdated
ivan-neverov-al and others added 6 commits May 19, 2025 14:42
ivan-neverov-al force-pushed the add_excluded_and_mode branch from 8ed36b9 to 1bbc954 Compare May 19, 2025 19:55
ivan-neverov-al requested a review from andrcuns May 19, 2025 19:56
andrcuns requested changes May 19, 2025
View reviewed changes

andrcuns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

One small thing I missed. I think we should not allow to create json with invalid values, it will cause issues downstream I believe

Comment thread allure-ruby-commons/lib/allure_ruby_commons/model/parameter.rb Outdated
Comment thread allure-ruby-commons/lib/allure_ruby_commons/model/parameter.rb Outdated
Comment thread allure-ruby-commons/spec/unit/allure_spec.rb Outdated

andrcuns commented May 19, 2025

Copy link
Copy Markdown
Collaborator

Sorry, some of my suggestions are causing rubocop issues, I don't have an editor with code open right now

ivan-neverov-al requested a review from andrcuns May 19, 2025 20:32
ivan-neverov-al force-pushed the add_excluded_and_mode branch from b3d3fbc to 404d994 Compare May 19, 2025 21:29

ivan-neverov-al commented May 19, 2025

Copy link
Copy Markdown
Contributor Author

@andrcuns sorry, missed rubocop and one test

qlty-cloud-legacy Bot commented May 20, 2025

Copy link
Copy Markdown

Code Climate has analyzed commit 404d994 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 98.9% (0.0% change).

View more on Code Climate.

andrcuns merged commit 44d90db into allure-framework:master May 20, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

andrcuns andrcuns approved these changes

Assignees

No one assigned

Labels

enhancement Improvement or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL