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

Updated github references to use https instead of git #240

Merged
merged 3 commits into from Jan 19, 2016

Conversation

robertjamesmiller
Copy link

We needed this change to support our dev server's proxy.

@robertjamesmiller
Copy link
Author

FYI: I needed to update app/models/cover_letter_sanitizer.rb so that delayed_jobs would start on our dev server. The code looked like it was trying to allow "em" tags but the test: spec/models/cover_letter_sanitizer_spec.rb was asserting the allowance of "p" tags, so I updated the code so that the test continued to pass.

class CoverLetterSanitizer < HTML::WhiteListSanitizer
self.allowed_tags = HTML::WhiteListSanitizer.allowed_tags - %w(em)
class CoverLetterSanitizer < Rails::Html::WhiteListSanitizer
self.allowed_tags = %w(p)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Robert, this doesn't have to do with switching git to https and shouldn't be in this pull request.

@amcates
Copy link
Collaborator

amcates commented Jan 6, 2016

I just noticed your comment. I thought it was removing 'em' tags from allowed_tags.

@robertjamesmiller
Copy link
Author

Ah, I now I see, was the old version subtracting em from allowed tags? The p tag was the only one being tested so things should work fine. Do you want me to make any adjustments to this pull request?

@amcates
Copy link
Collaborator

amcates commented Jan 6, 2016

I'm wondering if we even need this anymore. I believe Cover Letters can't be accessed in the new setup. Might just remove the "self.allowed_tags" all together.

@robertjamesmiller
Copy link
Author

Gotcha, we still need the "github references to use https instead of git" so maybe you accept the pull request and then take out the cover letter references and dependencies? Or, do you want me to do that?

@amcates
Copy link
Collaborator

amcates commented Jan 6, 2016

We'll take care of it

jleonardw9 added a commit that referenced this pull request Jan 19, 2016
Updated github references to use https instead of git
@jleonardw9 jleonardw9 merged commit efda874 into sparc-request:master Jan 19, 2016
ying-pbrc pushed a commit to PBRCDev/sparc-request that referenced this pull request May 15, 2017
…_options_box_reconfig

Jtm status options box reconfig
@chrisortman chrisortman deleted the update_gemfile_to_use_https branch July 19, 2017 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants