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

Setting goals for a 1.0.0 milestone #427

Closed
scottrhoyt opened this issue Jan 27, 2016 · 19 comments
Closed

Setting goals for a 1.0.0 milestone #427

scottrhoyt opened this issue Jan 27, 2016 · 19 comments
Labels
discussion Topics that cannot be categorized as bugs or enhancements yet. They require further discussions. wontfix Issues that became stale and were auto-closed by a bot.
Milestone

Comments

@scottrhoyt
Copy link
Contributor

/cc @realm/swiftlint

I think it might be good to start a discussion about creating a 1.0.0 release milestone. We could nominate outstanding issues, create new issues, and add to this milestone where appropriate. That way collaborators can have clear direction on how to get the project to that point, and users could vote for what features are most important to them. Obviously stabilizing the public interface of both swiftlint and SwiftLintFramework should be primary focuses.

Another consideration should be what a breaking change is? Do we consider introducing a new rule that defaults to on or changing the default parameters of an old rule to be breaking changes? How about removing or combining old rules? These changes should not break the CLI on one hand, however they have the potential to cause failing build scripts both locally and on CI machines via a swiftlint upgrade.

What do you guys think, or is this too early to discuss?

If we want to start this now, I would nominate the following issues to be part of a 1.0.0 release milestone:

@scottrhoyt scottrhoyt added the discussion Topics that cannot be categorized as bugs or enhancements yet. They require further discussions. label Jan 27, 2016
@daniel-beard
Copy link
Contributor

Something I've brought up in the past is that using swiftlint in an enterprise setting usually means that we want only a subset of rules to be enabled (i.e. a whitelist single source of truth that overrides opt-in and disabled rules lists).

The lack of such support has made upgrading swiftlint a real pain, as the rules often change and cause new unexpected violations. I would be happy to contribute something like this, and would hate to have to maintain a fork to achieve the same behavior.

Note, this was completed in #256

@jpsim
Copy link
Collaborator

jpsim commented Jan 29, 2016

Thanks for taking the initiative to bring this up, @scottrhoyt! This is something I've thought of occasionally but never really pinned down what it would mean for SwiftLint to be 1.0.

A few ideas:

  1. Trimming the public API. There are probably many public declarations that don't necessarily need to be. Removing those, or reducing their ACL will make it easier to maintain API stability post-1.0.
  2. Reviewing & finalizing the YAML configuration format. The nice thing about YAML is that you can add support for new keys without making breaking changes, but it'd be nice to make sure the config options that work today will continue to work for a little while after 1.0.
  3. I feel like Add version pinning à la Podfile.lock #221 might be a good candidate for 1.0 as well, although the need for that is somewhat lessened by Add whitelist only rules #436.
  4. SwiftLint Performance Benchmarking #376 would also be a good candidate for 1.0.

@ZevEisenberg
Copy link
Contributor

Is there an easy way to specify the version of SwiftLint to use per-project, like one might specify the version of a Pod? If so, that could be a good way to sidestep the issue of breaking changes, because you could just pin the version of SwiftLint per-project. Guessing that would have to be external, i.e. specifying a version of SwiftLint for brew to activate, but it could also be a feature of the tool itself: stick rules_version = 0.7.2 at the top of .swiftlint.yml, and any version of the tool released after that feature can activate and deactivate rules. Obviously, though, that would introduce more overhead in terms of maintaining rules perpetually, so it might be overkill.

@jpsim
Copy link
Collaborator

jpsim commented Jan 29, 2016

@ZevEisenberg yeah, I think that should be considered, which is why I linked to the issue you opened about this in my comment above (#221).

@scottrhoyt scottrhoyt added this to the 1.0.0 milestone Feb 3, 2016
@tamarnachmany
Copy link

I, like @daniel-beard would be interested in supporting a configuration that explicitly enables rules, rather than explicitly disabling rules, for exactly the same reason -- violations which are introduced in new versions of SwiftLint cause unanticipated errors, and may also reflect rules that we don't want implemented on our team.

I'd be happy to work on this if I know there is interest in this kind of change, or just to participate in the conversation.

@daniel-beard
Copy link
Contributor

@tamarnachmany there is now a new configuration in the .swiftlint.yml files called whitelist_rules that performs this behavior. See #256

@ZevEisenberg
Copy link
Contributor

@what about a model like GCC/Clang warnings and -Weverything: by default, everything is off and you switch them on one by one. But if you want, you can enable -Weverything and then disable the ones you don't want, so that you get a chance to evaluate new rules when they are introduced. If you're into that sort of thing.

@tamarnachmany
Copy link

When I tried to use that functionality - I just whitelisted the rules I want to lint for - some of the rules I did not explicitly disable but did not whitelist were still linted.

@daniel-beard
Copy link
Contributor

@tamarnachmany this is only present in the latest master builds, there has not been a release yet with this functionality. If this was happening from a source build from master, can you raise a ticket for it?

@tamarnachmany
Copy link

Hm. So it sounds like I saw that PR, added a whitelist to my config file, and then my config file must have been ignored since it was invalid (the configuration was invalid, since the whitelist functionality is unreleased). Cool. Looking forward to that change.

@marcelofabri
Copy link
Collaborator

marcelofabri commented Dec 31, 2016

Almost one year later, I'd like to share my thoughts on what should be done to reach 1.0 IMO:

Performance

These already have pending PRs, but should improve drastically SwiftLint performance

Documentation/Experience

Configuration handling

We have several opened issues on how configurations are handled: #1075, #987, #736, #676, #606, #551, #550, #554.

Stability

Bugs

There are lots of issues linked here but IMO most of the hardest ones are being tackled already. The ones that probably deserved an special attention are the ones related to configuration as no progress has been done on that area for a while.

@jpsim
Copy link
Collaborator

jpsim commented Jan 1, 2017

If API stability is a goal for 1.0 (which it should be IMHO), then conforming to the Swift 3 API Guidelines is also a must.

@jpsim
Copy link
Collaborator

jpsim commented Jan 10, 2017

The migration to conform to the Swift 3 API Guidelines was done in #1139, #1140, #1141, #1142, #1143, #1144 and #1145. Thanks @marcelofabri for the quick reviews.

@jpsim
Copy link
Collaborator

jpsim commented Jan 11, 2017

Cache implemented by @marcelofabri in #1073

@jpsim
Copy link
Collaborator

jpsim commented Nov 13, 2017

Checked off a few items from #427 (comment).

Big pieces left to do are (loosely):

  • Change cli arguments to be unix-compliant
  • Add config command with init and check options
  • Trim public API

I propose we push to get these done in the coming weeks. It'd be nice to finally mark SwiftLint as 1.0.

@marcelofabri
Copy link
Collaborator

I'd love to get some feedback about #1843 to see if it's something we want to do or not. If it is, IMO we should do it before 1.0.

@jpsim
Copy link
Collaborator

jpsim commented Nov 13, 2017

Never really took a good luck at that to be honest. Was a bit scared off by the size.

@marcelofabri
Copy link
Collaborator

I don't particularly care about the implementation, it's more about its goals and (breaking) changes.

@stale
Copy link

stale bot commented Nov 8, 2020

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!

@stale stale bot added the wontfix Issues that became stale and were auto-closed by a bot. label Nov 8, 2020
@stale stale bot closed this as completed Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Topics that cannot be categorized as bugs or enhancements yet. They require further discussions. wontfix Issues that became stale and were auto-closed by a bot.
Projects
None yet
Development

No branches or pull requests

6 participants