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 alerts api support #26

Merged
merged 12 commits into from
Oct 8, 2015
Merged

Added alerts api support #26

merged 12 commits into from
Oct 8, 2015

Conversation

jkandasa
Copy link
Member

@jkandasa jkandasa commented Oct 5, 2015

@vnugent I have added alerts api support. Kindly review and merge it

@jkandasa
Copy link
Member Author

jkandasa commented Oct 5, 2015

Ticket: Add support for Alert API #15

@jkandasa
Copy link
Member Author

jkandasa commented Oct 5, 2015

@vnugent Does Hawkular Demo server update to date?

@FilipB
Copy link

FilipB commented Oct 6, 2015

Just a few general comments:

  • some javadoc and comments would be definitely nice
  • is there any reason not to log anything?
  • not sure if this is doable everywhere but it's recommended to fix the code rather than using SuppressWarnings("unchecked")

@jkandasa
Copy link
Member Author

jkandasa commented Oct 6, 2015

@FilipB Thank you for your valuable feedback!

@vnugent
Copy link
Contributor

vnugent commented Oct 7, 2015

Can you add the failing tests to the known-failure group, eg @Test(groups={"known-failure"})?

@jkandasa
Copy link
Member Author

jkandasa commented Oct 7, 2015

@vnugent marked alerts failure test cases with @Test(groups={"known-failure"})

@vnugent
Copy link
Contributor

vnugent commented Oct 7, 2015

@jkandasa can you pull and rebase new changes I added to master? test in travis should pass with the new known-failure exclusion

vnugent added a commit that referenced this pull request Oct 8, 2015
@vnugent vnugent merged commit ff8ff1b into hawkular:master Oct 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants