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

Added Max Retries text field on Instance screen #3562

Merged
merged 3 commits into from
Mar 14, 2018

Conversation

h-kataria
Copy link
Contributor

@h-kataria h-kataria commented Mar 9, 2018

@h-kataria
Copy link
Contributor Author

@gmcculloug please review, fixes: #3560

@gmcculloug
Copy link
Member

@tinaafitz @mkanoor Please review.

@tinaafitz
Copy link
Member

@h-kataria Looks good.

@mkanoor
Copy link
Contributor

mkanoor commented Mar 12, 2018

@h-kataria
Can we add max_time also along with it since they go as a pair for state fields.
Also for non state fields can we disable those edit boxes, because they dont make sense for attributes, assertions and other field types.

@tinaafitz
Copy link
Member

@h-kataria There are 5 fields that are only applicable to state machines:
on_entry, on_exit, on_error, max_retries, and max_time.
Would it be possible to display these fields only in the case the class is a state machine class?
You can check the class to see if its a state machine (for example MiqAeClass.first.state_machine?)
Additionally, It would be great if you were able to allow those fields to be updated only if the field was a state. (It's confusing because a class is considered a state machine class if it contains a single entry of type state. It could still have other entries of attributes, methods, and relationships)

Also changed on_entry, on_exit, on_error, max_retries, and max_time fields to only show if class is a state class and also for state fields only see ManageIQ#3562 (comment) for further details.

Issue# 3560
@h-kataria
Copy link
Contributor Author

@tinaafitz @mkanoor please review, made requested enhancements.

@tinaafitz
Copy link
Member

@h-kataria The changes look great!

@h-kataria
Copy link
Contributor Author

@tinaafitz @mkanoor please review, made a change as per our conversation to make "Collect" field on the form disabled if field type is not state or relationship.

@miq-bot
Copy link
Member

miq-bot commented Mar 13, 2018

Checked commits h-kataria/manageiq-ui-classic@ce2d176~...ef95156 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Haml - Linter::Haml STDERR:
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

@gmcculloug
Copy link
Member

@h-kataria This is looking great, thanks.

@tinaafitz
Copy link
Member

@h-kataria Thanks for the additional change for the collect field. It looks great!
@mkanoor Please review.

@mkanoor
Copy link
Contributor

mkanoor commented Mar 13, 2018

@h-kataria looks great. 👍
You might want to remove the first after screen shots, it shows collect for attributes, which might confusing to documentation folks.

@h-kataria
Copy link
Contributor Author

@mkanoor updated first screenshot

@mkanoor
Copy link
Contributor

mkanoor commented Mar 14, 2018

👍

@dclarizio dclarizio merged commit 81d2462 into ManageIQ:master Mar 14, 2018
@dclarizio dclarizio added this to the Sprint 82 Ending Mar 26, 2018 milestone Mar 14, 2018
@h-kataria h-kataria deleted the add_max_retries_for_instances branch April 16, 2018 20:48
@gmcculloug
Copy link
Member

@dclarizio Would it be possible to make this gaprindashvili/yes?

Asking for a friend. @fdupont-redhat

@dclarizio
Copy link

Relabeled for gaprindashvili to address this RFE:
https://bugzilla.redhat.com/show_bug.cgi?id=1596338

@JPrause
Copy link
Member

JPrause commented Jun 29, 2018

@miq-bot add_label blocker

simaishi pushed a commit that referenced this pull request Jul 11, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit b50a100152bfd246630121d0e0da12b499e22e89
Author: Dan Clarizio <dclarizi@redhat.com>
Date:   Wed Mar 14 14:42:22 2018 -0700

    Merge pull request #3562 from h-kataria/add_max_retries_for_instances
    
    Added Max Retries text field on Instance screen
    (cherry picked from commit 81d2462db09f354460390d384af6f6948ef6545e)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1598528

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

Successfully merging this pull request may close these issues.

None yet

8 participants