FalckJoshua / DIT113-System-Development-Locus-Imperium

Other
0 stars 0 forks source link

implements #47 - [merged] #99

Closed FalckJoshua closed 7 months ago

FalckJoshua commented 1 year ago

In GitLab by @cjoshua on May 4, 2023, 14:35

Merges 47-set-the-broker-ip-address-within-in-the-app -> main

What does this MR do?

Adds the functionality to change broker IP address inside the application.

[Closes #47 #49]

Acceptance criteria met

FalckJoshua commented 1 year ago

In GitLab by @cjoshua on May 4, 2023, 14:35

requested review from @vasilena

FalckJoshua commented 1 year ago

In GitLab by @vasilena on May 4, 2023, 14:50

The code looks good! Same for naming & commenting.

Apart from that, what do you think about keeping the 'MQTT_SERVER' in 'BrokerConnection.java'? I think it can be initialized at the beginning, and then the value can be assigned in the constructor, like 'LOCALHOST'. What do you say?? @cjoshua :smile_cat: :ok_hand_tone1:

FalckJoshua commented 1 year ago

In GitLab by @cjoshua on May 4, 2023, 15:27

added 1 commit

Compare with previous version

FalckJoshua commented 1 year ago

In GitLab by @cjoshua on May 4, 2023, 15:30

@vasilena good feedback added variable name, also addressed #49.

FalckJoshua commented 1 year ago

In GitLab by @vasilena on May 4, 2023, 15:51

Glad to hear, ready to merge! :smile: :handshake_tone1:

FalckJoshua commented 1 year ago

In GitLab by @vasilena on May 4, 2023, 15:52

approved this merge request

FalckJoshua commented 1 year ago

In GitLab by @vasilena on May 4, 2023, 15:52

mentioned in commit 4aa2a46d40bdc6ff199019a25739f53a84ae16e3