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

modulesync 5.3 & update EoL URI syntax + a lot of rubocop rework #463

Merged
merged 15 commits into from
Nov 23, 2022
Merged

modulesync 5.3 & update EoL URI syntax + a lot of rubocop rework #463

merged 15 commits into from
Nov 23, 2022

Conversation

bastelfreak
Copy link
Member

No description provided.

@bastelfreak bastelfreak changed the title modulesync 4.20.0 modulesync 4.2.0 Sep 17, 2021
@bastelfreak bastelfreak changed the title modulesync 4.2.0 modulesync 4.2.0 & update EoL URI syntax Nov 23, 2021
@bastelfreak bastelfreak changed the title modulesync 4.2.0 & update EoL URI syntax modulesync 5.1.0 & update EoL URI syntax Dec 14, 2021
@bastelfreak bastelfreak changed the title modulesync 5.1.0 & update EoL URI syntax modulesync 5.1.0 & update EoL URI syntax + a lot of rubocop rework Dec 14, 2021
@bastelfreak bastelfreak force-pushed the modulesync branch 2 times, most recently from 38c86a9 to 56ea220 Compare December 14, 2021 20:53
@bastelfreak
Copy link
Member Author

this is mostly rubocop -a (safe autofix) and a lot of rework. Since this module contains a lot of types and providers, please review this carefully.

root-expert
root-expert reviewed Mar 20, 2022
View reviewed changes
spec/acceptance/authentication_spec.rb Outdated
require 'spec_helper_acceptance'
require 'uri'

context 'authenticated download' do
let(:source) do
URI.escape("http://httpbin.org/basic-auth/user/#{password}")
CGI.escape("http://httpbin.org/basic-auth/user/#{password}")
Copy link
Member

Choose a reason for hiding this comment

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

CGI.escape doesn't play well with URI.regexp (https://github.com/voxpupuli/puppet-archive/blob/master/lib/puppet/type/archive.rb#L127-L129)

I'm wondering if switching to normal regexp is an option?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am fine with whatever works :D

@vox-pupuli-tasks
Copy link

Dear @bastelfreak, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs?
If you need any help, you can reach out to us on our IRC channel #voxpupuli on Libera.Chat or our Slack channel voxpupuli at slack.puppet.com.
You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@bastelfreak bastelfreak changed the title modulesync 5.1.0 & update EoL URI syntax + a lot of rubocop rework modulesync 5.3 & update EoL URI syntax + a lot of rubocop rework Jul 21, 2022
@prolixalias
Copy link
Contributor

Hey there Tim... I resurrected this PR moments ago, rebased from a recent PR I'd submitted. Once it's through, is it of any use to you for me to try and wrap up #463 ?

@bastelfreak
Copy link
Member Author

@prolixalias thanks for the work, can you take a look at the failing tests?

@prolixalias prolixalias mentioned this pull request Nov 4, 2022
Merged
@prolixalias
Copy link
Contributor

prolixalias commented Nov 4, 2022 •
edited
Loading

@prolixalias thanks for the work, can you take a look at the failing tests?

Was able to get issues resolved for everything but archlinxrolling. The wget "not functional" part is throwing me for a loop though. Can you help me with that one?

@bastelfreak
Copy link
Member Author

It looks like all acceptance tests are failling, not only arch linux. because of:

Failure/Error: URI.escape("http://httpbin.org/basic-auth/user/#{password}") # rubocop:disable Lint/UriEscapeUnescape
NoMethodError:
  undefined method `escape' for URI:Module

I'm not sure whats the proper fix here. @alexjfisher I think you solved that in another module recently?

@prolixalias
Copy link
Contributor

It looks like all acceptance tests are failling, not only arch linux. because of:

Failure/Error: URI.escape("http://httpbin.org/basic-auth/user/#{password}") # rubocop:disable Lint/UriEscapeUnescape
NoMethodError:
  undefined method `escape' for URI:Module

I'm not sure whats the proper fix here. @alexjfisher I think you solved that in another module recently?

It's fixed in my PR475 Figured I'd rebase your branch from master when everything made it through.

@prolixalias
Copy link
Contributor

@bastelfreak - are any other modules giving you trouble for modulesync? If so, please give me a list and I'll work on them over the weekend

@bastelfreak bastelfreak merged commit 6f26da1 into master Nov 23, 2022
@bastelfreak bastelfreak deleted the modulesync branch November 23, 2022 10:48
@bastelfreak bastelfreak added enhancement New feature or request and removed tests-fail labels Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@root-expert root-expert root-expert left review comments

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bastelfreak @prolixalias @root-expert @trevor-vaughan