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

Deprecate collection #624

Merged
merged 4 commits into from
Oct 14, 2024

Conversation

mariolenz
Copy link
Contributor

@felixfontein
Copy link
Collaborator

Should we deprecate all content as well? Or just hope the users somehow notice this from the changelogs / README?

Context: I wrote the following on matrix:

05:10:59 <Felix Fontein> I would add a deprecation marker to the README, and maybe even deprecate all content in the collection, so that users know that the collection will stop living soon (so potential maintainers for parts of it can step up, otherwise they might simply miss the announcements)

@Andersson007
Copy link
Contributor

Should we deprecate all content as well? Or just hope the users somehow notice this from the changelogs / README?

Context: I wrote the following on matrix:

05:10:59 <Felix Fontein> I would add a deprecation marker to the README, and maybe even deprecate all content in the collection, so that users know that the collection will stop living soon (so potential maintainers for parts of it can step up, otherwise they might simply miss the announcements)

makes sense to me

@mariolenz
Copy link
Contributor Author

Deprecate all modules, plugins and so on in meta/runtime.yml one by one or in a changelog fragment or both?

There's no way to deprecate the collection as a whole in meta/runtime.yml or is there? But maybe it's OK to have just a single entry to deprecate the collection in the changelog... what do you think?

I would do the work, but need some help what to do and about the wording.

@mariolenz
Copy link
Contributor Author

mariolenz commented Oct 11, 2024

I don't really understand meta/runtime.yml, there seems to be a lot of outdated stuff. I think we should clear this up first.

#625 is a first attempt to do it, but there might be other outdated stuff we could remove to make deprecating the collection easier.

@mariolenz mariolenz marked this pull request as draft October 11, 2024 15:44
@mariolenz
Copy link
Contributor Author

mariolenz commented Oct 12, 2024

I've tried to deprecate everything automatically in meta/runtime.yml like this:

import os
import yaml

from copy import deepcopy

categories = ['action', 'cliconf', 'doc_fragments', 'httpapi', 'lookup', 'modules', 'module_utils', 'netconf', 'terminal']
deprecation = {'removal_version': '6.0.0', 'warning_text': 'This collection and all content in it is unmaintained and deprecated.'}

with open('meta/runtime.yml') as file:
  runtime_ori = yaml.safe_load(file)

for cat in categories:
  if cat not in runtime_ori['plugin_routing'].keys():
    runtime_ori['plugin_routing'][cat] = {}
  subdir = os.path.join('plugins', cat)
  subdir_len = len(subdir) + 1
  for path, subdirs, files in os.walk(subdir):
    for name in files:
      if name.startswith('_'):
        continue
      content = os.path.join(path, name)[subdir_len:-3].replace('/', '.')
      if content not in runtime_ori['plugin_routing'][cat].keys():
        runtime_ori['plugin_routing'][cat][content] = {'deprecation': deepcopy(deprecation)}

print(yaml.dump(runtime_ori, default_flow_style=False))

@mariolenz mariolenz force-pushed the deprecate_collection branch 3 times, most recently from d657942 to 01f09d6 Compare October 12, 2024 12:58
@mariolenz
Copy link
Contributor Author

recheck

@mariolenz
Copy link
Contributor Author

Should we deprecate all content as well? Or just hope the users somehow notice this from the changelogs / README?

@felixfontein It turned out that there's a lot of content to deprecate 😫

Anyway, what do you think about my latest commits? I didn't add a changelog entry / fragment yet, though.

BTW I think the CI failures are unrelated. But I'm not 100% sure.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI failures are related to the deprecation in the plugins/modules.

There seems to be something strange with meta/runtime.yml, but maybe it's just GitHub's diff viewer. Will take a look later when I have more time.

plugins/cliconf/aireos.py Outdated Show resolved Hide resolved
@mariolenz
Copy link
Contributor Author

There seems to be something strange with meta/runtime.yml, but maybe it's just GitHub's diff viewer. Will take a look later when I have more time.

Maybe the way that I've used to generate meta/runtime.yml (see my Python code above) reordered some things and that's what looks weird to you? I don't know, this is just an idea.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@felixfontein
Copy link
Collaborator

There seems to be something strange with meta/runtime.yml, but maybe it's just GitHub's diff viewer. Will take a look later when I have more time.

Maybe the way that I've used to generate meta/runtime.yml (see my Python code above) reordered some things and that's what looks weird to you? I don't know, this is just an idea.

I think it was a GitHub bug (or I looked wrong). I didn't have much time and found some entries which were now wrong, but looking at it again they seem to have vanished, and when looking at the runtime.yml from a checkout it looks fine.

The main problem with the diff is that also existing entries were re-ordered, which is totally fine, but makes the diff more complex.

But now that I had more time I checked in more detail, and everything looks fine to me.

@mariolenz mariolenz marked this pull request as ready for review October 13, 2024 13:08
@mariolenz
Copy link
Contributor Author

Thanks for reviewing @felixfontein!

What do you think about this PR @Andersson007?

@Andersson007 Andersson007 added the backport-5 Automatically create a backport for the stable-5 branch label Oct 14, 2024
@Andersson007 Andersson007 merged commit f2c3554 into ansible-collections:main Oct 14, 2024
39 checks passed
@Andersson007
Copy link
Contributor

@mariolenz great work, thanks a lot!

Andersson007 pushed a commit to Andersson007/community.network that referenced this pull request Oct 14, 2024
* Deprecate collection

* Deprecate content in meta/runtime.yml

* Update documentation

* alternatives -> alternative

(cherry picked from commit f2c3554)
@ansible-collections ansible-collections deleted a comment from patchback bot Oct 14, 2024
Andersson007 pushed a commit to Andersson007/community.network that referenced this pull request Oct 14, 2024
* Deprecate collection

* Deprecate content in meta/runtime.yml

* Update documentation

* alternatives -> alternative

(cherry picked from commit f2c3554)
@Andersson007
Copy link
Contributor

@felixfontein @mariolenz so i guess now i need to release the collection, should we do it with an announcement under deprecated_features:, i.e. the fragment will be

deprecated_features:
- This collection and all content in it is unmaintained and deprecated (https://forum.ansible.com/t/8030). If you're interested in maintaining parts of the collection, please copy them in your own repository. See the collection creator path for details https://docs.ansible.com/ansible/devel/dev_guide/developing_collections_path.html.

How about ^ ?

Andersson007 added a commit that referenced this pull request Oct 14, 2024
* Deprecate collection

* Deprecate content in meta/runtime.yml

* Update documentation

* alternatives -> alternative

(cherry picked from commit f2c3554)

Co-authored-by: Mario Lenz <m@riolenz.de>
@mariolenz mariolenz deleted the deprecate_collection branch October 14, 2024 14:31
@mariolenz
Copy link
Contributor Author

Yes, I think having this information in the changelog makes sense. Also your proposal sounds good to me.

I've created #627 to add a changelog fragment.

@felixfontein felixfontein added backport-5 Automatically create a backport for the stable-5 branch and removed backport-5 Automatically create a backport for the stable-5 branch labels Oct 14, 2024
Copy link

patchback bot commented Oct 14, 2024

Backport to stable-5: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply f2c3554 on top of patchback/backports/stable-5/f2c35544d327c6d837ecb78a6392d3c1228b7508/pr-624

Backporting merged PR #624 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible-collections/community.network.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-5/f2c35544d327c6d837ecb78a6392d3c1228b7508/pr-624 upstream/stable-5
  4. Now, cherry-pick PR Deprecate collection #624 contents into that branch:
    $ git cherry-pick -x f2c35544d327c6d837ecb78a6392d3c1228b7508
    If it'll yell at you with something like fatal: Commit f2c35544d327c6d837ecb78a6392d3c1228b7508 is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x f2c35544d327c6d837ecb78a6392d3c1228b7508
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Deprecate collection #624 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-5/f2c35544d327c6d837ecb78a6392d3c1228b7508/pr-624
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein
Copy link
Collaborator

Hmm, looks like automatically backporting fails.

@felixfontein
Copy link
Collaborator

Ah, I see that this was manually backported in #626. Thanks @Andersson007!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-5 Automatically create a backport for the stable-5 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants