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.

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

Add hammer-cli-foreman-host-reports #7620

Conversation

ofedoren
Copy link
Member

Supported Versions:

  • Nightly
  • 3.2

@zjhuntin
Copy link
Contributor

[test deb]

1 similar comment
@zjhuntin
Copy link
Contributor

zjhuntin commented Mar 3, 2022

[test deb]

Comment on lines 12 to 13
Vcs-Git: https://salsa.debian.org/ruby-team/ruby-hammer-cli-foreman-host-reports.git
Vcs-Browser: https://salsa.debian.org/ruby-team/ruby-hammer-cli-foreman-host-reports
Copy link
Member

Choose a reason for hiding this comment

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

this isn't true, so let's not set those options :)

Comment on lines 4 to 5
Maintainer: Debian Ruby Extras Maintainers <pkg-ruby-extras-maintainers@lists.alioth.debian.org>
Uploaders: <>
Copy link
Member

Choose a reason for hiding this comment

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

this isn't true, as this package is not in debian, please set to yourself, or some generic foreman address.

@@ -0,0 +1,4 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

you can drop this file, it;s unused in our case

@evgeni
Copy link
Member

evgeni commented Mar 7, 2022

tests fail with File does not exist: minitest-spec-context -- missing dependency?

@ofedoren ofedoren force-pushed the deb/develop-hammer-cli-foreman-host-reports-0.1.0 branch from b6786ee to 7f90e0d Compare March 7, 2022 17:57
@ofedoren
Copy link
Member Author

ofedoren commented Mar 7, 2022

tests fail with File does not exist: minitest-spec-context -- missing dependency?

Well, my assumption was that since it's required in test files only and this gem is installed with hammer-cli-foreman's Gemfile (https://github.com/theforeman/hammer-cli-foreman/blob/master/Gemfile#L11) I could simply require it in h-c-f-host-reports as well.

Please don't tell me that now I need to update Gemfile/gemspec and release a new version just because of this...

@evgeni
Copy link
Member

evgeni commented Mar 7, 2022

tests fail with File does not exist: minitest-spec-context -- missing dependency?

Well, my assumption was that since it's required in test files only and this gem is installed with hammer-cli-foreman's Gemfile (https://github.com/theforeman/hammer-cli-foreman/blob/master/Gemfile#L11) I could simply require it in h-c-f-host-reports as well.

Please don't tell me that now I need to update Gemfile/gemspec and release a new version just because of this...

That I didn't say.

The packaging of hammer-cli-foreman doesn't depend on minitest-spec-context (and, it seems, doesn't execute any tests at all). Given m-s-c isn't packaged in Debian, I think the easiest would be to not execute tests here too, but I am not exactly sure what triggers them.

@@ -0,0 +1,6 @@
require 'gem2deb/rake/testtask'
Copy link
Member

Choose a reason for hiding this comment

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

can you try dropping this file and see what happens? I think that's the one that executes the tests for us.

@ofedoren ofedoren force-pushed the deb/develop-hammer-cli-foreman-host-reports-0.1.0 branch from 7f90e0d to 966d438 Compare March 8, 2022 09:29
@ofedoren
Copy link
Member Author

ofedoren commented Mar 8, 2022

ok to test

Maintainer: Oleh Fedorenko <ofedoren@redhat.com>
Uploaders: <>
Build-Depends: debhelper-compat (= 12),
gem2deb (>= 1),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
gem2deb (>= 1),
gem2deb,

we don't need >=1 and it's not available in buster

Section: ruby
Priority: optional
Maintainer: Oleh Fedorenko <ofedoren@redhat.com>
Uploaders: <>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Uploaders: <>

@@ -0,0 +1,9 @@
---
Archive: GitHub (FIXME)
Copy link
Member

Choose a reason for hiding this comment

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

What's the FIXME part here?

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea what this file is, anyways ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

https://wiki.debian.org/UpstreamMetadata#Fields doesn't tell much and I didn't find an example there :(

@ofedoren ofedoren force-pushed the deb/develop-hammer-cli-foreman-host-reports-0.1.0 branch from 966d438 to 75ab740 Compare March 9, 2022 10:48
@ofedoren
Copy link
Member Author

ofedoren commented Mar 9, 2022

@evgeni, all seems to be OK now :)

@evgeni evgeni merged commit 93f6c3b into theforeman:deb/develop Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants