Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clone os-autoinst-common into os-autoinst-distri-example #32

Merged
merged 1 commit into from
Feb 12, 2024
Merged

Clone os-autoinst-common into os-autoinst-distri-example #32

merged 1 commit into from
Feb 12, 2024

Conversation

b10n1k
Copy link
Contributor

@b10n1k b10n1k commented Feb 9, 2024 •
edited by kalikiana
Loading

Martchus
Martchus requested changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look right to add these cache files.

@b10n1k
Clone external/os-autoinst-common
0c5c726 
subrepo:
  subdir:   "external/os-autoinst-common"
  merged:   "db78c61"
upstream:
  origin:   "https://github.com/os-autoinst/os-autoinst-common.git"
  branch:   "master"
  commit:   "db78c61"
git-subrepo:
  version:  "0.4.6"
  origin:   "???"
  commit:   "???"
Signed-off-by: ybonatakis <[email protected]>
@b10n1k
Copy link
Contributor Author

b10n1k commented Feb 9, 2024

It doesn't look right to add these cache files.

fixed. i have no idea what happened in the previous commit

@b10n1k b10n1k requested a review from Martchus February 9, 2024 23:13
okurz
okurz reviewed Feb 12, 2024
View reviewed changes
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow it does not feel right to pull in much more content into this comparing to the minimal repo content already there from before. Maybe we should including the content using git submodules here nevertheless?

@foursixnine
Copy link
Member

Somehow it does not feel right to pull in much more content into this comparing to the minimal repo content already there from before. Maybe we should including the content using git submodules here nevertheless?

+1

@kalikiana
Copy link
Member

kalikiana commented Feb 12, 2024 •
edited
Loading

Somehow it does not feel right to pull in much more content into this comparing to the minimal repo content already there from before. Maybe we should including the content using git submodules here nevertheless?

git sub-repo which is used here is the approach taken in other repos. poo#138416 says in AC1 os-autoinst/os-autoinst-distri-example inherits static checks from os-autoinst-common. That's what this PR does.

@kalikiana kalikiana merged commit 6f25725 into os-autoinst:main Feb 12, 2024
8 checks passed
@okurz
Copy link
Member

okurz commented Feb 12, 2024

Somehow it does not feel right to pull in much more content into this comparing to the minimal repo content already there from before. Maybe we should including the content using git submodules here nevertheless?

git sub-repo which is used here is the approach taken in other repos. poo#138416 says in AC1 os-autoinst/os-autoinst-distri-example inherits static checks from os-autoinst-common. That's what this PR does.

I know that and likely you know that I know that so I don't see the point of your comment

b10n1k pushed a commit to b10n1k/os-autoinst-distri-example that referenced this pull request Feb 12, 2024
@kalikiana @b10n1k
Merge pull request os-autoinst#32 from b10n1k/update_repo
62e0d33 
Clone os-autoinst-common into os-autoinst-distri-example

Signed-off-by: ybonatakis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@okurz okurz okurz left review comments

@kalikiana kalikiana kalikiana approved these changes

@Martchus Martchus Martchus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@b10n1k @foursixnine @kalikiana @okurz @Martchus