WIP: feature/installcheck #99

Draft
atartamo wants to merge 8 commits from atartamo/autogits:feature/installcheck into main
Owner
No description provided.
atartamo added 3 commits 2025-11-12 15:33:37 +01:00
atartamo added 1 commit 2025-11-12 17:14:09 +01:00
atartamo added 1 commit 2025-11-13 10:30:20 +01:00
atartamo added 1 commit 2025-11-13 18:27:38 +01:00
atartamo added 1 commit 2025-11-19 13:50:35 +01:00
adamm requested changes 2025-11-21 12:43:12 +01:00
adamm left a comment
Owner

Thanks for the PR. Just a few nit-picks mosly and the regex missing the .

Thanks for the PR. Just a few nit-picks mosly and the regex missing the `.`
@@ -276,0 +278,4 @@
}
type InstallcheckConfig struct {
Repos map[string][]string `json:"repos"`
Owner

This needs documentation. This probably needs to be extended

https://src.opensuse.org/git-workflow/autogits/src/branch/main/obs-staging-bot#configuration-file

Need example of how these things are defined. map of what to what. string is not very descriptive here.

This needs documentation. This probably needs to be extended https://src.opensuse.org/git-workflow/autogits/src/branch/main/obs-staging-bot#configuration-file Need example of how these things are defined. map of what to what. `string` is not very descriptive here.
@@ -283,1 +292,4 @@
QA []QAConfig
Repositories map[string]RepositoryConfig `json:",omitempty"`
Installcheck InstallcheckConfig `json:",omitempty"`
Owner

pointer here, *InstallcheckConfig, would be omitted if empty.

map[string]*RepositoryConfig

pointer here, *InstallcheckConfig, would be omitted if empty. map[string]*RepositoryConfig
@@ -0,0 +15,4 @@
* PARTICULAR PURPOSE. See the GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along with
* Foobar. If not, see <https://www.gnu.org/licenses/>.
Owner

Foobar? :-)

Foobar? :-)
@@ -0,0 +6,4 @@
"strings"
)
// ParseSubprojectChangesFromDiff parses a diff string and returns a slice of package names that have changed.
Owner

slice of package names that have changed

should be

slice of repository names that have changed

not packages, repositories. For packages we need to parser the .gitmoduels and substitute the submodule mount point to repo name.

fortunately only few exceptions here, like the packages with +

> slice of package names that have changed should be > slice of repository names that have changed not packages, repositories. For packages we need to parser the .gitmoduels and substitute the submodule mount point to repo name. fortunately only few exceptions here, like the packages with `+`
@@ -0,0 +13,4 @@
// This regex finds diff chunks for subprojects.
// It looks for a `diff --git` line, followed by lines indicating a subproject commit change.
re := regexp.MustCompile(`diff --git a\/(.+) b\/(.+)\n`)
Owner

This probably needs ^ and $ to mark the front end end of the line.

This probably needs `^` and `$` to mark the front end end of the line.
@@ -0,0 +28,4 @@
const SkipSrcRpm = true
func ParseNotificationToPR(thread *models.NotificationThread) (org string, repo string, num int64, err error) {
rx := regexp.MustCompile(`^https://src\.(?:open)?suse\.(?:org|de)/api/v\d+/repos/(?<org>[-_a-zA-Z0-9]+)/(?<project>[-_a-zA-Z0-9]+)/issues/(?<num>[0-9]+)$`)
Owner

Need to add \. to the org and project names, in case those have a . in them.

^https://src\.(?:open)?suse\.(?:org|de)/api/v\d+/repos/(?<org>[-\._a-zA-Z0-9]+)/(?<project>[-\._a-zA-Z0-9]+)/issues/(?<num>[0-9]+)$

maybe better, this duplication from the staging bot can be put as a const in common/consts.go and references from both places.

TODO: this could be a URL parser and just fetch parts of the path as org, project and issue number.

Need to add `\.` to the org and project names, in case those have a . in them. `^https://src\.(?:open)?suse\.(?:org|de)/api/v\d+/repos/(?<org>[-\._a-zA-Z0-9]+)/(?<project>[-\._a-zA-Z0-9]+)/issues/(?<num>[0-9]+)$` maybe better, this duplication from the staging bot can be put as a const in `common/consts.go` and references from both places. TODO: this could be a URL parser and just fetch parts of the path as org, project and issue number.
@@ -0,0 +230,4 @@
changedPackages := common.ParseSubprojectChangesFromDiff(diff)
if len(changedPackages) > 0 {
common.LogInfo("Changed packages found in PR!")
Owner

Here we already have this functionality not by parsing a diff, but at looking at changes to the submodules. But maybe this is better if subdirectories need to be taken into account.

func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleList map[string]string, err error) {

(so nothing to change here, just a comment)

Here we already have this functionality not by parsing a diff, but at looking at changes to the submodules. But maybe this is better if subdirectories need to be taken into account. https://src.opensuse.org/git-workflow/autogits/src/commit/cc675c1b249a4a86eaf85f70ffa8caaa432b9e14f046247eb83c1d780499628f/common/git_utils.go#L865 (so nothing to change here, just a comment)
atartamo added 1 commit 2025-11-26 14:13:03 +01:00
This pull request has changes conflicting with the target branch.
  • common/config.go
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u feature/installcheck:atartamo-feature/installcheck
git checkout atartamo-feature/installcheck
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: git-workflow/autogits#99
No description provided.