Skip to content

Conversation

German22pb
Copy link

No description provided.

@Phoenix124
Copy link
Contributor

@svasenkov можете плиз проревьюить и смержить, нужная фича

Copy link
Collaborator

@valfirst valfirst left a comment

Choose a reason for hiding this comment

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

My personal recommendation is to split this PR into 2: 1 for rocket chat and 1 for testOps

Comment on lines +30 to +31
implementation('io.rest-assured:rest-assured:4.4.0')
implementation('org.jboss.resteasy:resteasy-jackson2-provider:3.0.6.Final')
Copy link
Collaborator

Choose a reason for hiding this comment

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

are these dependencies used somewhere?

}
catch (JsonSyntaxException e) {
JsonReader reader = new JsonReader(new StringReader(json));
reader.setLenient(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this tweak needed?

@@ -19,14 +21,23 @@
public class Chart {

public static byte[] createChart(Base base) throws MessageBuildException {
return createChart(base, null);
}
public static byte[] createChart(Base base, TestOps testOps) throws MessageBuildException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

the thing here is generic Chart starts "knowing" about TestOps, which is not good if low coupling is followed

- [x] [Email config](https://github.com/qa-guru/allure-notifications/wiki/Email-configuration)
- [x] [Skype config](https://github.com/qa-guru/allure-notifications/wiki/Skype-configuration)
- [x] [Mattermost config](https://github.com/qa-guru/allure-notifications/wiki/Mattermost-configuration)
- [x] [Rocket config]

Choose a reason for hiding this comment

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

broken link

@@ -117,6 +155,11 @@ Languages: 🇬🇧 🇫🇷 🇷🇺 🇺🇦 🇧🇾 🇨🇳
}
}
```
You only need:
- to fill needed options in `base` block (please, be careful, `language` field is required!);
- to configure desired destinations for notifications (`telegram`, `slack`, `mattermost`, `skype`, `mail`), keep in mind it's possible to set multiple destinations at once, if no destination is set, then no notification will be sent and no error will occur;

Choose a reason for hiding this comment

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

add rocket

@exmike exmike mentioned this pull request Nov 18, 2023
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.

4 participants