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

Removing reference to Valkyrie on 2.x branch #4247

Merged
merged 2 commits into from
Feb 2, 2020

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Feb 2, 2020

  • Ensuring CSV usage has same namespace consideration
  • Removing reference to Valkyrie on 2.x-stable branch

These two commits, which don't effect the production version of Hyrax, do impact the test suite for Hyrax development on the 2.x-stable branch.

@samvera/hyrax-code-reviewers

This change came as part of some PRs/merging that happened upstream.

As a result, it broke the build.
This is a bit of a stab in the dark. However, we had a failed Ruby
build with the following message:

```console
1) Hyrax::FileSetCSVService when specifying terms and separator csv
     Failure/Error:
       ::CSV.generate do |csv|
         csv << terms.map do |term|
           values = file_set.send(term)
           values = values.respond_to?(:to_a) ? values.to_a : [values] # make sure we have an array
           values.join(multi_value_separator)
         end
       end

     NameError:
       uninitialized constant CSV
```

What I suspect to be in play is that the corresponding spec does not
similarly namespace the `CSV` constant. The production ruby file uses
`::CSV` whereas the spec ruby file uses `CSV`. This commit seeks to use
the same namespacing, hopefully reducing an intermittent error.

See the following  CircleCI build for the failures:

-  https://circleci.com/gh/samvera/hyrax/17063#tests/containers/3
@jeremyf jeremyf requested a review from no-reply February 2, 2020 14:24
@jcoyne jcoyne merged commit 1682ae0 into 2.x-stable Feb 2, 2020
@jcoyne jcoyne deleted the fixing-broken-2.7-build branch February 2, 2020 17:52
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.

2 participants