Conversation
Pull request artifacts
|
|
This makes more sense in the gogsbridge. Can you redo the functionality for the gogsbridge and add changes to the gitea bridge if neccessary? I dont know if gogs-tags work different to gitea-tags. It's hard to find any public instances with actual data and releases. |
|
Hello Bockiii, If you follow the link in the OP, you'll see that this feature was implemented in Gitea, independently of Gogs. You can compare both implementation of tags here : I don't think there is enough in common between the two to warrant a solution that works for both, implemented in GogsBridge. |
|
We at least need to remove all the duplicate code. Right now you basically copied the whole gogs bridge into the gitea bridge. |
|
I'm not a big fan of inheritance. But if we follow bockiis advice and do it properly, it should be done like this: diff --git a/bridges/GiteaBridge.php b/bridges/GiteaBridge.php
index 3324787..25165aa 100644
--- a/bridges/GiteaBridge.php
+++ b/bridges/GiteaBridge.php
@@ -24,4 +24,17 @@ class GiteaBridge extends GogsBridge {
);
}
}
+
+ protected function collectTags($html)
+ {
+ $tags = $html->find('table#tags-table > tbody > tr')
+ or returnServerError('Unable to find tags');
+
+ foreach($tags as $tag) {
+ $this->items[] = array(
+ 'uri' => $tag->find('a', 0)->href,
+ 'title' => 'Tag ' . $tag->find('.release-tag-name > a', 0)->plaintext,
+ );
+ }
+ }
}
diff --git a/bridges/GogsBridge.php b/bridges/GogsBridge.php
index a2adc1f..0167dab 100644
--- a/bridges/GogsBridge.php
+++ b/bridges/GogsBridge.php
@@ -53,6 +53,7 @@ class GogsBridge extends BridgeAbstract {
),
),
'Releases' => array(),
+ 'Tags' => array(),
);
private $title = '';
@@ -98,6 +99,7 @@ class GogsBridge extends BridgeAbstract {
case 'Issues':
case 'Releases': return $this->title . ' ' . $this->queriedContext;
case 'Single issue': return $this->title . ' Issue ' . $this->getInput('issue');
+ case 'Tags': return 'Tags for ' . $this->title;
default: return parent::getName();
}
}
@@ -127,6 +129,9 @@ class GogsBridge extends BridgeAbstract {
case 'Releases': {
$this->collectReleasesData($html);
} break;
+ case 'Tags':
+ $this->collectTags($html);
+ break;
}
}
@@ -202,4 +207,9 @@ class GogsBridge extends BridgeAbstract {
);
}
}
+
+ protected function collectTags($html)
+ {
+ // Not implemented for gogs
+ }
}
|
The code is duplicated for a reason : the projects are drifting away more and more, and some features are not available in Gogs, or Gitea (e.g. the Tags page). We could do more convoluted inheritance between the two bridges, but to what end ? Both bridges are simple enough anyway.
@dvikan Thank you for suggesting an implementation ! However doing it this way makes the Tags context available in the GogsBridge in the web UI, making users think it's implemented. Is using inheritance more important than presenting the correct available contexts to users ? I think we away with it, especially if you're not a fan of it too ;) Also your comment made me realize that the Bridge did not have a name/title for the Tags context, that's fixed in f758993. |
|
Your comments make sense gilleri. I think I prefer duplication here. So let's do it properly. Duplicate the bridge, drop the inheritance and implement the desired changes. |
|
I'm not a big fan of inheritance either, but if we do it, we should do it correctly :) So yes, if we assume that both projects are drifting away further, please rewrite the gitea bridge so it doesn't inherit anything and just stands on it's own. Worst case scenario would be a change that needs to be done in both bridges, that's doable. |
|
I agree, thanks for your support. I've rewritten completely GiteaBridge to decouple it from GogsBridge, and will open a new PR, since this one strayed quite far from the original goal. |
Following feedback in #2069, here is a new PR, with a more limited scope.
Add support for tags context.