Page MenuHomeSolus

Fix a build issue of the plugin by applying an upstream commit.
AbandonedPublic

Authored by sankasan on Jan 4 2018, 9:14 PM.

Details

Reviewers
JoshStrobl
Group Reviewers
Triage Team
Summary
Test Plan
  1. Connect to a network using the network-manager-fortisslvpn plugin.
  2. Ping several computers in the network and notice the response.
  3. Use remmina to setup a RDP connection to one of the servers and see a working remote desktop session.
  4. Disconnect and see that step 2 and 3 are no longer possible.

Diff Detail

Repository
R3699 network-manager-fortisslvpn
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

sankasan created this revision.Jan 4 2018, 9:14 PM
JoshStrobl requested changes to this revision.Jan 6 2018, 6:59 PM
JoshStrobl added a subscriber: JoshStrobl.
  1. You have duplicate information in your summary.
  2. You need to provide a test plan (how you tested the software)
This revision now requires changes to proceed.Jan 6 2018, 6:59 PM
sankasan updated this revision to Diff 4508.Jan 6 2018, 8:18 PM
sankasan edited edge metadata.
This comment was removed by sankasan.
sankasan edited the summary of this revision. (Show Details)Jan 6 2018, 8:21 PM
sankasan edited the test plan for this revision. (Show Details)
sankasan retitled this revision from Fix the build by applying an upstream commit. to Fix a build issue of the plugin by applying an upstream commit..
sankasan edited the summary of this revision. (Show Details)

Those duplicate messages come from given information in both git and arc. I'd skip the latter in the next patches.

Ready to review.

JoshStrobl accepted this revision.Jan 18 2018, 4:53 PM
This revision is now accepted and ready to land.Jan 18 2018, 4:53 PM
JoshStrobl requested changes to this revision.Jan 18 2018, 4:56 PM

Actually you need to rebase your commit on master. Your change is using rel 5 when we're on rel 6 currently.

This revision now requires changes to proceed.Jan 18 2018, 4:56 PM
sankasan updated this revision to Diff 4830.EditedJan 22 2018, 4:30 PM
sankasan edited edge metadata.
sankasan edited the test plan for this revision. (Show Details)

Updated the patch to the latest revision.

JoshStrobl requested changes to this revision.Jan 27 2018, 12:50 PM

So a few things:

  1. 1.2.8 is now out that has this fix: https://download.gnome.org/sources/NetworkManager-fortisslvpn/1.2/NetworkManager-fortisslvpn-1.2.8.news
  2. You've been incrementing your relnum each time you've submitted a diff. That isn't necessary. A 1.2.8 update should be rel 6.
This revision now requires changes to proceed.Jan 27 2018, 12:50 PM

A short reply to your notes. I'm not the maintainer of this package and (try to) help by fixing a broken package by submitting a patched build script. Rejecting the patch three times doesn't inspire me to do this more frequent. Especially as the initial patch was valid at the time of submission and the changes requested after were reactions to updates to the package by others.

Anyway, you can close this one as I did a regular update using the update script for 1.2.8 (rel 7) which you can find here: https://dev.solus-project.com/D2150

JoshStrobl abandoned this revision.EditedJan 30 2018, 5:04 PM

Especially as the initial patch was valid at the time of submission and the changes requested after were reactions to updates to the package by others.

Except it wasn't valid. Your relnum was still inaccurate and in need of a rebase, which you still have not done, as I stated on your other diff.