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

Allow to sort pages by created_at and title #323

Closed
wants to merge 1 commit into from

Conversation

igor-drozdov
Copy link

@igor-drozdov igor-drozdov commented Mar 6, 2019

Hello,

I would like to propose the changes for sorting pages by created-at and title. It requires files_sorted_by_created_at to be implemented in adapters (I have implemented in the rugged one gollum/rugged_adapter#32).

Please, let me know what do you think about this functionality and the solutions (if you have better ideas for the solution, I am willing to discuss and work on it)

@bartkamphorst
Copy link
Member

@igor-drozdov Sorry for the slow reply. I think this could be quite a useful addition to gollum. @dometto What do you think?

@dometto
Copy link
Member

dometto commented Apr 9, 2019

I'm not against being able to sort pages in various ways, but I am a bit curious what the intended purpose of this is. It adds some complications to the logic of the lib and adapter, and am not sure yet for what reason. Is this to be able to sort in different ways in gollum? In that case, why not just pass a hash with all the info on the files (path, creation date, size) and allow sorting based on that info in the UI?

I am also not sure whence the need for the new public method on Repo in adapter, and the new public method on GitAccess. Wouldn't it make more sense to add the sorting options to Repo#ls_files and keep the rest of the API as is?

@dometto
Copy link
Member

dometto commented Apr 9, 2019

Incidentally, this PR should be made against the gollum-5.x branch

@dometto
Copy link
Member

dometto commented Apr 28, 2019

Closing for now, please feel free to reopen against 5.x and explain the points I raised above

@dometto dometto closed this Apr 28, 2019
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.

3 participants