mcci-catena / docker-iot-dashboard

A complete IoT server for LoRaWAN IoT projects: node-red + influxdb + grafana + ssl + let's encrypt using docker-compose.
MIT License
101 stars 59 forks source link

Installing Specific package #11

Closed MuruganChandrasekar closed 3 years ago

MuruganChandrasekar commented 6 years ago

As per your advice, it's been changed. please review it.

MuruganChandrasekar commented 6 years ago

Sure, I'll try to review it by today.

MuruganChandrasekar commented 6 years ago

Hello Terry,

I've changed the overall influxdb back up setup and pushed my changes. Please review it.

terrillmoore commented 6 years ago

@MuruganChandrasekar did you notice that there's a conflict? I suggest that you rebase your pull request as follows:

  1. git checkout master
  2. git fetch upstream (assuming you have upstream pointing to mcci-catena).
  3. git merge upstream/master (this should get your master in sync with mcci-catena)
  4. git checkout extra-instance-for-influxdb-backup
  5. git rebase master (for alternatives see below)
  6. (very important: rename the branch so you can push). git branch -m influxdb-backup
  7. git push to your repo
  8. submit new pull request with the new branch.

Alternately you can merge at step 5, and then you should be able to push to your existing branch, which will update here. The branchname is no longer descriptive, but that would also work. That will be much easier to review.

terrillmoore commented 6 years ago

Unfortunately, I still can't merge this. There are conflicts that you must resolve -- see https://github.com/mcci-catena/docker-ttn-dashboard/pull/11 and scan to the bottom of the conversation, and you'll see "This branch has conflicts that must be resolved". In fact, it looks like you missed an important correction from the community: you cannot default to localhost.com -- that's not valid. You must default mail addresses to localhost (without a top-level domain). Please correct this and get to the point where there are no more conflicts to be resolved. Thanks! --Terry

terrillmoore commented 6 years ago

@MuruganChandrasekar did you see this message?

Unfortunately, I still can't merge this. There are conflicts that you must resolve -- see #11 and scan to the bottom of the conversation, and you'll see "This branch has conflicts that must be resolved". In fact, it looks like you missed an important correction from the community: you cannot default to localhost.com -- that's not valid. You must default mail addresses to localhost (without a top-level domain). Please correct this and get to the point where there are no more conflicts to be resolved. Thanks! --Terry

I'm happy to help on this, but it's really much better if we coordinate efforts. Thanks! --Terry

terrillmoore commented 6 years ago

@MuruganChandrasekar it's possible that you can't see the conflict report. I tried looking at #11 from the a browser that wasn't logged in, and I couldn't see the conflict report.

image

Here are the conflicts in detail: image

Hopefully this helps understand my comments. --Terry

MuruganChandrasekar commented 6 years ago

Hello Terry,

Sorry for the delayed response, I changed as you wished, but you have to change the below since we are versioning for all the packages installed.

image

I think, here in the below image, naming variable was wrong so please change it. image