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

Fixes #12885 - Remove forcing 'no whitespace' from policy name #147

Merged
merged 1 commit into from Aug 23, 2016
Merged

Fixes #12885 - Remove forcing 'no whitespace' from policy name #147

merged 1 commit into from Aug 23, 2016

Conversation

shlomizadok
Copy link
Member

No description provided.

@shlomizadok
Copy link
Member Author

Will fail on engine error.... :/

ohadlevy
ohadlevy reviewed Dec 20, 2015
View reviewed changes
app/models/foreman_openscap/policy.rb
@@ -21,7 +21,7 @@ class Policy < ActiveRecord::Base
SERVER_CLASS_PARAMETER = 'server'
PORT_CLASS_PARAMETER = 'port'

validates :name, :presence => true, :uniqueness => true, :format => { :without => /\s/ }
validates :name, :presence => true, :uniqueness => true
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason why this existed in the first place? any limitation on scap scanner itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not as far as I know. And we use this name only to display it on foreman.
Maybe in the past it was used to set the puppet? We switched to id long time ago though

@shlomizadok
Copy link
Member Author

[test]

@ares
Copy link
Member

ares commented Jan 19, 2016

the test is failing because of this change, seems like it was on purpose

@shlomizadok
Copy link
Member Author

@ares, not sure [test] was unstable. (also tested locally and it worked)

@ares
Copy link
Member

ares commented May 20, 2016

let's see again but I think the [test] would have to be fixed too, IIRC it was no intermittent failure

@theforeman-bot
Copy link
Member

There were the following issues with the commit message:

  • 65230d5 must be in the format fixes #redmine_number - brief description
  • 05ad882 must be in the format fixes #redmine_number - brief description
  • 05b5e84 must be in the format fixes #redmine_number - brief description
  • 4876975 must be in the format fixes #redmine_number - brief description
  • b4f471c must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project.

More guidelines are available in Coding Standards or on the Foreman wiki.

This message was auto-generated by Foreman's prprocessor

@theforeman-bot
Copy link
Member

@shlomizadok, this pull request is currently not mergeable. Please rebase against the master branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream master

This message was auto-generated by Foreman's prprocessor

@xprazak2
Copy link
Contributor

@shlomizadok could you rebase pls? I will set this as WoC for now...

@shlomizadok
Copy link
Member Author

shlomizadok commented Aug 22, 2016 •
edited

@xprazak2 - rebased and fixed the test :)

@xprazak2
Copy link
Contributor

Works for me, we can merge if there are no objections.

@ares
Copy link
Member

ares commented Aug 23, 2016

👍

@shlomizadok shlomizadok merged commit ea2230f into theforeman:master Aug 23, 2016
@shlomizadok
Copy link
Member Author

Thanks @ares & @xprazak2 for reviewing and testing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Ready to merge
Projects
None yet
Milestone
No milestone
5 participants
@shlomizadok @ares @theforeman-bot @xprazak2 @ohadlevy