Hello, Ayushman; On Fri, Jan 12, 2024 at 01:55:15AM +0530, Ayushman Tripathi wrote: > Hello! I'm reaching the final stage of the process of implementing the > option for the Git webpage repository. > This patch basically covers the PHP changes.
What about the other end, linking the checkout of the repository
to website and updating it on the web server?
> -function form_radio ($name, $value, $attr)
> +function form_radio ($name, $value, $attr = [])
The default value is never used in your patch, and in fact,
form_radio makes little sense without at least a label, which has
no reasonable default.
> - db_execute ("SELECT * FROM groups WHERE group_id = ?", [$id]);
> + db_execute ("SELECT * FROM `groups` WHERE group_id = ?", [$id]);
Current Savane code doesn't use this kind of quoting; irrespectively
of whether they may improve the program, it seems to me it's
pointless to introduce this kind of inconsistency in an unrelated
modification.
> + function isArtifactUrlSet ($artifact)
> + {
> + # Returns true if the artifact url is set for this group.
> + return $this->data_array["url_$artifact"] != "";
> + }
This function is never used, and I don't think it would be really
helpful.
> + function getHomepageVcs ()
> + {
> + return $this->data_array['homepage_vcs'];
> + }
The code implies a new field in the groups table, and in many
cases of access to data of the group it will not be used.
That alone is a good reason against it, and the compatibility
issues are even more important.
It's OK to keep this value in $this->data_array, but the code to
extract and update it should be different.
> @@ -743,7 +754,7 @@ function group_get_artifact_url ($artifact, $hostname = 1)
> global $project, $sys_home;
> $type_urls = [
> "homepage", "download", "cvs_viewcvs", "cvs_viewcvs_homepage",
> - "arch_viewcvs", "svn_viewcvs", "git_viewcvs", "hg_viewcvs", "bzr_viewcvs"
> + "arch_viewcvs", "svn_viewcvs", "git_viewcvs", "git_viewcvs_homepage",
> "hg_viewcvs", "bzr_viewcvs"
Savane follows GNU recommendations for style. Please keep
the length of source lines to 79 characters or less,
https://www.gnu.org/prep/standards/html_node/Formatting.html
Also, these VCS URLs are in fact the same for all group types; we'd
better decommission these useless fields that make group types
unmanageable rather than add yet one more.
> - if (!pagemenu_url_is_set ($group, "cvs_viewcvs_homepage"))
> + if (!pagemenu_url_is_set ($group, "cvs_viewcvs_homepage") &&
> !pagemenu_url_is_set ($group, "git_viewcvs_homepage"))
The group will use no more than one VCS for its homepage; a new
field 'url_git_viewcvs_homepage' will only add unneeded complexity.
signature.asc
Description: PGP signature
