-
Notifications
You must be signed in to change notification settings - Fork 667
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
Remove lint command #3802
Remove lint command #3802
Conversation
It's a good idea to slim the project. It seems lint dependencies is missed. https://github.com/ansible-community/molecule/blob/main/pyproject.toml#L68-L74 |
@zhan9san there are few missing bits as I just started but "lint" extra is not to be removed as its purpose is for internal use. What is funny is that even if we remove it, people will not observe because pip ignores any missing extras. |
Wouldn't it be a good idea to put up a deprecation warning for a few versions before removing the option entirely? |
Did this really need to be removed? Why can't a general By removing the ability for |
Agreed! I guess a possible workaround now, to be able to keep the linting within molecule and not relay on any external scripts, would be to add the linting into one of those playbooks. |
@gardar @davedittrich My general experience is that once Sorin has put something into place that's the end of it (even if most of us on the outside are surprised), so we'll just have to figure out how to live with this change that breaks >95% of CI / CD workflows in the real world. In the end, not a big deal...just an unexpected major annoyance. |
Yeah, don't expect any communication or reasoning. A workaround might be to use the "shell" type dependency to run |
Molecule's workflow came about from legitimate "real-world" use-cases in Ansible role development. Built from the trenches, at a time |
This is in preparation for Molecule 5.x, which removes the `lint` command. For more information, see: ansible/molecule#3802 Aside from that, this approach is also a bit nicer since linter errors can be identified by their own GitHub checks run, which should be easier for developers compared to the previous approach of having to hunt through Molecule's error logs for linter errors. Also, it's a bit more efficient since we only run the linters once per CI run instead of in every Molecule scenario.
This is in preparation for Molecule 5.x, which removes the `lint` command. For more information, see: ansible/molecule#3802 Aside from that, this approach is also a bit nicer since linter errors can be identified by their own GitHub checks run, which should be easier for developers compared to the previous approach of having to hunt through Molecule's error logs for linter errors. Also, it's a bit more efficient since we only run the linters once per CI run instead of in every Molecule scenario.
So, what's the alternative now? |
@madrover At $dayjob we'll likely be implementing an additional linting stage to the CI/CD flow that gets run on every push / merge request / and merge operation on our ansible repositories. Many places use git-hooks, both pre- and post-commit to enforce linting (but please read the manual before doing that, for the sake of your sanity). Additionally, many editors these days support on-the-fly linting via plugins (EG: VSCode and the dearly departed Atom). There are other viable options, this change was just rather unexpected outside of the core maintainer group. |
For a short explanation of why lint was removed see here: #3825 (reply in thread) |
Thanks for the insights. I understand that this can be easily overcome on a CI/CD workflow and/or using other techniques, although it's annoying to have to tweak all the existing pipelines, but we also use Molecule intensively as a local development tool and having the linting capability built in was very nice. I can understand the rationale described at #3825 (reply in thread) but removing all the functionality altogether is probably not the most thoughtful approach for the users. IMHO, this functionality could have been disabled by default, thus solving the described issues, but left there for users that rely on it. In a nutshell, while I don't disagree with the rationale of not running linting, I think that making it toggleable and disabled by default would have achieved the same result and would have been less onerous for users. |
From what I have seen in digging through code, I believe the linter worked perfectly fine until an attempt was made to produce fancy colored output, which didn't do encoding properly (hence the "c h a r a c t e r s i n t h e o u t p u t" looked funny). Multiple attempts to fix this, or detect I also noticed that this discussion involved In summary, the pace of changes in this (and related) projects seems to be too high, since a non-trivial number of those changes produce regression problems, give the user no time to deal with deprecations, or introduce latent bugs that add to maintenance costs later on. I'd prefer fewer changes, more carefully designed and following a test-driven development methodology that exposes regression problems earlier, and working more closely with people contributing PRs or capable of multi-repo debugging. |
Signed-off-by: Valerii Svydenko <[email protected]>
We are asking frequently. |
I spent hours trying to debug my molecule installation before I landed here. There are still plenty of resources which hint that such feature exists and when searching for the official docs, it returns some results, from the count of which you may guess linting is not there anymore, but I feel like it would be very good to have something - anything confirming that this doesn't exist. This issue is already quite buried in github so I think finding this information out is difficult. Just to provide context, I was quickly able to find some examples of configuring molecule linting from web search so it is totally reasonable someone else has experienced this frustration: Also, on molecule version 6, if one goes and adds the I'm just getting started with molecule, while I do not like linting not being there anymore, I do understand the reasoning here. Additionally I feel like this thread was at least somewhat informational resource for finding a workaround, so it would provide value as linked to the docs if some mention is at some point added. Otherwise we have to pray for favorable SEO. |
Molecule docs could really be improved to indicate that this has been deprecated. Searched the web for an hour or so before I landed here... |
See ansible/molecule#3802 @todo: Find a replacement for molecule lint.
…supported versions (#16) * fix: 24.04 support * fix: actions and molecule tweaks * fix: action version * fix: namespace and role name * Remove outdated versions * Update versions and images * Match molecule with one that has working tests * Welp. So much for linting with molecule. See ansible/molecule#3802 @todo: Find a replacement for molecule lint. * Downgrade molecule-action version to see if that solves it * Switch to an image that actually exists * Woops * Try a few more things * Try a few more things * Try things Robert's way * Fix lint later * Fix wrong requirements file * Noble doesn't exist yet. * Add some more requirements * Fix broken boostrap * Duh * Update Mariadb service name change transition version * Adjust lower * Find out why service is hanging * Last attempt for mariadb service timing out on focal * Add forgotten flush * Disable focal from molecule tests. * Enable molecule on cron * Noble is supported. Yay. * Remove unneeded stuff * Update min requirement --------- Co-authored-by: Jaden Seniuk <[email protected]>
See ansible/molecule#3802 and ansible-community#346 for alternatives.
* Fix molecule working dir See gofrolist/molecule-action#60. * Remove lint as it is no longer part of molecule See ansible/molecule#3802 and #346 for alternatives. * Install the community.general collection Install the community.general collection, because rhsm_repository used in install_hashi_repo.yml is from there. * Delete older distributions that fail CI
No description provided.