Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    ceilingfish
    @ceilingfish
    I've pushed some changes to that.
    While we work out the details of that PR, is dotliquid/dotliquid#351 worth a bit of discussion?
    ceilingfish
    @ceilingfish
    Could we add something to RenderParameters to collect errors in thread safe mode?
    ceilingfish
    @ceilingfish
    Have tried this out in #466
    ceilingfish
    @ceilingfish
    My apologies for the mass of PRs today, my employer gave us the day to work on open source, and given we use dotliquid quite alot, I thought I'd try and make some contributions back.
    microalps
    @microalps
    @ceilingfish I appreciate the work!
    microalps
    @microalps
    Re: errors I made a note in the task. I should also note that the name or RenderParameters is a little counter-intuitive to putting responses back in. Having a new method like your TryRender would be good, although Try implies a boolean response. Maybe a new class RenderStreamParameters with a class response object with errors etc.
    Re: increment I made a small comment should be an quick fix
    microalps
    @microalps
    Re: cancellationToken I need to take a good look to understand the relationship between context and render parameters before we delve into solutions.
    I would think that off the cuff, render parameters which are options should not include something like a cancellation token. Same options can be used across multiple runs but cancellation token would be different. It does make sense in a context somewhat
    microalps
    @microalps
    Apologies if I contradicted myself I'm quite confused. Need to put my thoughts on paper.
    microalps
    @microalps
    Property RenderParameters <br />Global RenderParameters <br />Thread-Safe Context RenderParameters <br />With Context
    Context NULL NULL N/A X
    Filters X [^1] Discarded Via AddFilters Discarded
    ErrorsOutputMode X X X
    SyntaxCompatibilityLevel X X X
    MaxIterations X X X
    FormatProvider X X X
    Timeout X X Marked Deprecated Restarted
    Registers X [^2] Discarded X Discarded
    LocalVariables X X Environments[0]
    Errors X X
    CancellationToken X -
    1. Template class calls AddFilters
    2. Template class merges global registers with instance ones
    microalps
    @microalps
    This is the state I see right now. What's definitely clear to me is that context does not belong in RenderParameters and Render method itself should accept Context directly. That would be a breaking change, but otherwise the API is totally confusing.
    microalps
    @microalps
    @daviburg I'm probably missing something in my research - I did this purely off GitHub without verifying anything in VS. A refactor would probably make this easier to understand and eventually document on the wiki.
    David burg
    @david_burg_twitter
    Hi folks. A heads-up that I am going offline to use much unused carry-over vacations days before they vanish at the end of the calendar year.
    So if I'm unresponsive to PR review requests, it's because I'm enjoying baguette, foie gras and wine in France!
    And cheese, don't forget the many cheeses.
    Sébastien Ros
    @sebastienros
    And the local choucroute I bet. Enjoy!
    microalps
    @microalps
    @david_burg_twitter @tgjones My latest PR didn't kick off any CI. Did something change recently?
    Tim Jones
    @tgjones
    Looks like there's a problem on GitHub's end, with the webhook: https://www.githubstatus.com/incidents/tyc8wpsgr2r8
    microalps
    @microalps
    Strange that it lets me merge though without the webhook working . Oh well.
    microalps
    @microalps
    It kicked off and CI + codecov came through. Thanks @tgjones
    @david_burg_twitter @daviburg needs your stamp of approval
    David burg
    @david_burg_twitter
    Where do I need to approve?
    microalps
    @microalps
    code review on my PR please @david_burg_twitter
    dotliquid/dotliquid#475 - not sure if all the permutations are overkill
    David burg
    @david_burg_twitter
    Review done. Minor code cosmetic suggestions on the test code.
    microalps
    @microalps
    Applied changes, that was easy. Will merge once CI is done.
    microalps
    @microalps
    Error publishing package. NuGet server returned 403: The specified API key is invalid, has expired, or does not have permission to access the specified package.
    @tgjones you last updated this Feb 2021. I assume it had a 1 yr lifetime. Please update again...
    microalps
    @microalps
    ping @tgjones
    Tim Jones
    @tgjones
    I've regenerated the NuGetKey, and updated appveyor.yml with (the encrypted version of) it.
    Tim Jones
    @tgjones
    Yeah, seems like it's worked.
    microalps
    @microalps
    Thanks Tim!
    microalps
    @microalps
    @david_burg_twitter what are your thoughts about #472
    Anusha Karaka
    @AnushaKaraka
    @david_burg_twitter @tgjones code review on my PR please dotliquid/dotliquid#477
    microalps
    @microalps
    @AnushaKaraka didn't see any push to address my PR review. If you fix, I can merge.
    7 replies
    David burg
    @david_burg_twitter
    Reviewed
    microalps
    @microalps
    @david_burg_twitter dotliquid/dotliquid#478 is ready for your review as discussed yesterday
    David burg
    @david_burg_twitter
    👍 reviewed.
    microalps
    @microalps
    What is your thought about sharing the regex? Where?
    microalps
    @microalps
    Pushed my changes. Let me know if we can merge.
    microalps
    @microalps
    I added a new issue dotliquid/dotliquid#481 as @tgjones doesn't seem to get gitter notifications consistently. It is a followup to conversations I had privately with @laedit and trying to resolve.
    David Burg
    @daviburg
    Is Tim Jones the only person with access to the domain name and hosting? Anyone else has shared admin access to either? (Is there an 'at all' tag for gitter to broadcast?)
    David Burg
    @daviburg
    @tgjones could you nominate additional project owners for DotLiquid at https://github.com/orgs/dotliquid/people so they can do project settings maintenance?
    Tim Jones
    @tgjones
    Hi @daviburg - I've given you and @microalps admin permissions for the DotLiquid org.
    David Burg
    @daviburg
    Thank you, much appreciated. I do see the project settings page now.
    David Burg
    @daviburg
    Please review: dotliquid/dotliquid#484
    microalps
    @microalps
    @daviburg What do you think about dotliquid/dotliquid#265 - this is due to drop returning true on ContainsKey always. It is needed to allow BeforeMethod to execute. How should we address? Pretty annoying. Separate issue is an easier way to capture those errors in thread safe mode, but one issue at a time.
    David Burg
    @daviburg
    Hi. I'm not familiar or do not remember at the moment what BeforeMethod takes care of.
    Also... starting an on-call rotation in a few hours, so I will be quite unresponsive, sorry.
    Would be good to add your thoughts on the github issue 'though, to add that increment of figuring it out.