Git Merge Request #363: Threaded comments do not include URL params (merged)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

Guillermo Cruz wants to merge 2 commits from /u/guillermocruz/allura/ to master, 2021-08-27

Fixed comments links by setting the limit and page values from url params

Commit Date  
[363944] (thread-url-params) by Guillermo Cruz Guillermo Cruz

fixed code that was redefining the variable params. share url includes params only if page is not 0

 2021-08-27 18:19:16 Tree
[2593b5] by Guillermo Cruz Guillermo Cruz

page and limit url params are now being included in threaded comments links

 2021-08-18 17:05:51 Tree

Discussion

  • Dave Brondsema

    Dave Brondsema - 2021-08-27

    Nice. Just some minor feedback for improvement

    • if page is 0 that parameter can be omitted. Same for limit. That'll make the URLs a bit simpler and cleaner
    • params is defined & redefined which is a little confusing. Maybe don't do set params = request.params and then do request.params.get(.. when needed
     

  • Dave Brondsema

    Dave Brondsema - 2021-08-27
    • Status: open --> merged
     

Log in to post a comment.