by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Kai Cataldo
    @kaicataldo
    First item:

    TSC Summary

    This PR proposes to include the Node version when hashing the cache key, ensuring that the cache is busted when consecutive linting runs are executed with different Node versions.

    TSC Question

    Should we accept this?

    Kevin Partington
    @platinumazure
    (As a note, Ilya and I have :+1:ed that comment)
    Toru Nagashima
    @mysticatea
    :+1:
    Kai Cataldo
    @kaicataldo
    :+1:
    Brandon Mills
    @btmills
    :+1:
    Kai Cataldo
    @kaicataldo
    Great! Resolution is to accept this proposal.
    Next item:

    TSC Summary

    In this comment, I noted that it is currently considered a breaking change to add warnings for any reason (including outside of lint rules), because of eslint --max-warnings=0.

    Our semver policy currently states that causing build errors on any enhancement should always be treated as semver-major (and this is correct and sensible). When someone is using --max-warnings, any new warnings can cause a build error.

    At the same time, warnings are currently our best way of reporting non-fatal errors from core (e.g., if there are minor issues with disable comments, deprecated rules, etc.). Errors and warnings are fundamentally different: Errors change the exit code of CLI processes and generally represent issues that users must resolve, but warnings do not change the exit code of CLI processes and generally represent minor issues that users could ignore. Warnings only become issues that users must resolve if they use --max-warnings in the CLI, and that is an opt-in decision.

    TSC Question

    Can we consider adding new warnings in semver-minor enhancements, as long as they are only warnings? As part of this, can we consider --max-warnings and any CLIEngine equivalent as the user bypassing our semver policy, similar to eslint:all?

    If not, could we design a new way to communicate to users, in such a way that RFCs which add information for users could do so without semver risks? For example:

    • New message severity that can never trigger nonzero exit code even with --max-warnings
    • A new report section for informational messages unrelated to per-file linting
    Kevin Partington
    @platinumazure
    This discussion is semi-blocking eslint/rfcs#34 right now, as a note. So it would be wonderful to have a direction
    Kai Cataldo
    @kaicataldo
    I think the biggest issue here is that we conflate linting warnings with other warnings at the moment
    Seems like introducing more linting warnings in rules should be treated the same way we do changes that increase errors reported in rules, but I agree that console warnings around feature deprecation or tool usage can be useful and should not have to be treated as a breaking change.
    Brandon Mills
    @btmills
    We've had several times where we wished we could report something to users that wasn't necessarily from a lint rule
    Toru Nagashima
    @mysticatea
    Increasing warnings for errored stuff is fine. But, increasing warnings for existing features (i.e., runtime-deprecation) in minor releases is not good to me.
    Kevin Partington
    @platinumazure
    Yeah... If we decide to keep lint warnings and deprecation warnings in the same array, I'd like to differentiate our policy on adding more lint messages vs adding more deprecation warnings
    Kai Cataldo
    @kaicataldo
    Sure, I'm just saying I think there's value in being able to log information to users - maybe that was a bad example
    Configuration issues, etc. are definitely more the use case I'd like to support (agreed we shouldn't spam deprecation warnings)
    Toru Nagashima
    @mysticatea
    Then, to me, warning on // global/// eslint is a runtime-deprecation.
    Kevin Partington
    @platinumazure
    Sorry, I spoke badly. Okay, we have lint warnings and configuration warnings. Let's ignore deprecation warnings for now. I'm more concerned about introducing warning-only configuration messages, and having that be restricted to semver-major only under the current policy.
    Kai Cataldo
    @kaicataldo
    So, just to be clear, are we trying to come to consensus on the issue brought up in that RFC?
    Agreed with @platinumazure
    Kevin Partington
    @platinumazure
    Yeah, let's focus on the RFC's concern please. I might have broadened the issue inappropriately
    Kai Cataldo
    @kaicataldo
    @mysticatea Do you mind expanding on your point above?
    FWIW, I'm supportive of introducing warnings in semver-minor releases
    Toru Nagashima
    @mysticatea
    I'm sorry, I'm failing translation English...
    Kai Cataldo
    @kaicataldo
    No problem!
    And updating our semver policy to include that
    Though I think it would be useful in the future to try to come up with a better way to distinguish between warnings caused by a linting run and those that are just trying to provide information to the user
    @btmills Would love to hear your thoughts
    Toru Nagashima
    @mysticatea
    If it's adding warnings to improve UX, I'm fine to do in minor releases.
    Brandon Mills
    @btmills
    I'd also be supportive of changing the semver policy to allow us to add configuration warnings in semver-minor releases. To me, the damage from someone unknowingly having the tool do something different than what they intended is greater than having to fix it during an upgrade
    Sounds like I agree with @platinumazure and @kaicataldo
    Kai Cataldo
    @kaicataldo
    Sounds like we're all generally supportive of this idea
    Do we want to hash this out in an issue? Or try to resolve it now?
    Toru Nagashima
    @mysticatea
    If the new warning appears in current normal usage, I think we should be careful.
    Kai Cataldo
    @kaicataldo
    I think the remaining question is how we define what kinds of warnings we're going to allow in semver-minor releases
    Brandon Mills
    @btmills
    I'd also be supportive of a distinct message type that separates core messages from lint messages, but that seems like a long-term solution, and I'm okay with this solution we could have immediately
    Kai Cataldo
    @kaicataldo
    Sounds like we should hash this out in an issue
    Kevin Partington
    @platinumazure
    I'm okay with that. :+1: I have a proposal that I can post in an issue.
    Kai Cataldo
    @kaicataldo
    So maybe accept this proposal with the caveat that we need to come up with a final design?
    Brandon Mills
    @btmills
    :+1:
    Kai Cataldo
    @kaicataldo
    :+1:
    Kevin Partington
    @platinumazure
    :+1:
    Kai Cataldo
    @kaicataldo
    @mysticatea Thoughts?
    Toru Nagashima
    @mysticatea
    :+1:
    Kai Cataldo
    @kaicataldo
    Resolution is to accept this proposal and to discuss the details in an issue that @platinumazure will create
    Next item: