-
Notifications
You must be signed in to change notification settings - Fork 0
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
Release/v4.0.0 #269
Closed
Closed
Release/v4.0.0 #269
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
other work in this commit: split facet translation functionality into own method basic code cleanup to match styleguide recommendations v1.0 version sends facets with just key: value v1.1 (future) will send key: { source: , num: } backwards compatible: - value_label deprecated but forwards input to new method so apps that overrode the search result items or item show metadata pages are OK largely not so much backwards compatible: - if entire files were overridden like facets and browse_facets then instead of a num the app will display a hash
remaining code should be API agnostic as far as display and queries apps which have overwritten helpers may encounter errors
I dunno what I was thinking when I wrote this code, because looking at it today I couldn't figure out why I thought that was a good idea. Must have just not been paying attention. Also fixes comment which used the wrong value Robocop agrees that it is bad. https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Lint/CircularArgumentReference
it's an odd bug...considering we've used the paginator a lot but I suspect it was due to various parameters I was plugging in trying to get some deprecation warnings to show up in query.rb
matches up with each other for one thing, less confusing for another
keyword facets now returning with markup of this kind but without sanitize that is just being hidden
does not go into as much depth about rvm lists dependency information
there are some TODOs left in here, but it's taking shape!
not sure that this is the way that it will ultimately be but I took some broad strokes at it
I left out some parts with high complexity, or explaining files like application.scss and application.js, which I feel are somewhat self-documenting. We can add that back if wanted!
other work in this commit: split facet translation functionality into own method basic code cleanup to match styleguide recommendations v1.0 version sends facets with just key: value v1.1 (future) will send key: { source: , num: } backwards compatible: - value_label deprecated but forwards input to new method so apps that overrode the search result items or item show metadata pages are OK largely not so much backwards compatible: - if entire files were overridden like facets and browse_facets then instead of a num the app will display a hash
remaining code should be API agnostic as far as display and queries apps which have overwritten helpers may encounter errors
matches up with each other for one thing, less confusing for another
keyword facets now returning with markup of this kind but without sanitize that is just being hidden
Will and I met on Zoom to discuss a number of things. These are my notes I prepared for the conversation with a couple edits with information Will shared. We decided it's best to close this and make a new PR with just the newer commits at the end of this branch. My notes:
|
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
renaming this branch, also I want to merge the later branches into this one