Skip to content

Conversation

@sromerotech
Copy link

I've added the vhost parameter so you can customize it. I'll appreciate any feedback on this PR. Thanks in advance.

Copy link
Member

@AlvaroVega AlvaroVega left a comment

Choose a reason for hiding this comment

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

LGTM

| IOTA_MQTT_PROTOCOL | mqtt.protocol |
| IOTA_MQTT_HOST | mqtt.host |
| IOTA_MQTT_PORT | mqtt.port |
| IOTA_MQTT_VHOST | mqtt.vhost |
Copy link
Member

Choose a reason for hiding this comment

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

As far as I understand, the new vhost parameter is for AMQP configuration, not MQTT (looking to "twin" PR telefonicaid/iotagent-json#527 it only appears in the AMQP section of the configuration).

Is this a typo? Or maybe I'm missing something? :)

Copy link
Author

Choose a reason for hiding this comment

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

Hello @fgalan and sorry for the delay. Yes, it was a typo. I've updated the PR. Thanks.

@@ -0,0 +1,7941 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

This file should be removed from PR.

@@ -1,5 +1,5 @@
name: CI
Copy link
Member

Choose a reason for hiding this comment

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

Why this file has been changed in the PR?

In addition, looking to telefonicaid/iotagent-json#527 I don't see similar changes

@fgalan
Copy link
Member

fgalan commented Apr 13, 2021

There were some changes on that iotagent-node-lib library that make some tests to be "unaligned". It has been already fixed in master (by PR #476 ).

Thus, could you please update this PR brach con master. After that I guess that all your changes in ci.yml in this PR are not needed and can be reverted. Remember also to remove packages-lock.json from the PR. Thanks!

@sromerotech sromerotech closed this by deleting the head repository May 11, 2024
@fgalan
Copy link
Member

fgalan commented May 13, 2024

@sromerotech could you explain why this PR has been closed, please? Do you plan to re-create it? Thanks!

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