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

Add inventory alicloud_ecs #145

Closed
wants to merge 1 commit into from

Conversation

lixue323
Copy link
Contributor

SUMMARY

Add plugin alicloud_ecs

COMPONENT NAME

inventory/alicloud_ecs

@ansibullbot ansibullbot added affects_2.10 new_contributor Help guide this first time contributor new_plugin New plugin stale_ci CI is older than 7 days, rerun before merging needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Apr 10, 2020
@lixue323 lixue323 force-pushed the invt branch 2 times, most recently from 5acfde6 to 82374fe Compare April 10, 2020 07:33
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.

Adding "Request changes" review to make sure this isn't merged before #108

Also, some kind of tests would be good. Maybe some integration tests (marked as unsupported)?

@lixue323
Copy link
Contributor Author

Adding "Request changes" review to make sure this isn't merged before #108

Also, some kind of tests would be good. Maybe some integration tests (marked as unsupported)?

@felixfontein I didn't really understand the meaning of the last sentence, I guess it was for me to write a test case, should I understand it that way

@felixfontein
Copy link
Collaborator

We started with a new rule for new plugins and modules in ansible/ansible last year that says that every new plugin/module must have tests (at least unit tests or integration tests; integration tests are allowed to be marked as unsupported, i.e. they are not run automatically in CI, if they require a special environment, such as accounts at cloud providers). IMO this should also be true for new plugins/modules in community.general and community.network.

BTW, unrelated to this, but since you created a large amount of new module/plugin PRs: have you thought about making a dedicated collection for alicloud? That has two big advantages:

  1. You are independent from any restrictions on community.general, and you are free to set up your own (internal?) CI for the collection.
  2. You can release your collection whenever you think a release is needed, and you can merge PRs at your own will.

(CC @gundalow)

@lixue323
Copy link
Contributor Author

@felixfontein I want to know the difference between making a collection under community like community.aws, community.network and making a collection elsewhere?

@felixfontein
Copy link
Collaborator

@lixue323 I think the difference is that community.* collections are not driven by a company, but by unrelated people from the community. For the alicloud modules, it probably makes sense to have them in a collection alibaba.alicloud. @gundalow are there some details I'm forgetting?

@lixue323
Copy link
Contributor Author

  1. how to create a repository alibaba.alicloud in Ansible Collections
  2. I need to publish them to galaxy later, right?
  3. About the documentation, I do nothing, I make collection alibaba.alicloud under collections, the documentation will be automatically generated in ansible doc, right?

@felixfontein @gundalow Sorry i have a lot of questions because i am confused

@gundalow gundalow self-assigned this Apr 14, 2020
@gundalow
Copy link
Contributor

@lixue323 Hi,
We recommend that when someone wants to add a group of new modules they should generally go into their own collection. If that collection is owned by a company (rather than a set of unrelated community members) then the repo should be in the organizations GitHub repo.

So I suggest you create your collection under https://github.com/alibaba

@lixue323
Copy link
Contributor Author

@gundalow I think you are right, but I still have a question. How to make the subsequent alicloud modules’s documentation into the official ansible documentation https://docs.ansible.com/ansible/latest/modules/list_of_cloud_modules.html

@gundalow
Copy link
Contributor

@gundalow I think you are right, but I still have a question. How to make the subsequent alicloud modules’s documentation into the official ansible documentation https://docs.ansible.com/ansible/latest/modules/list_of_cloud_modules.html

Hi,
We don't have an answer to that today, though we should do in the next few weeks and months.

Given you've said you are happy to move to a Collection I'll close the other PRs.

@ansibullbot ansibullbot added stale_ci CI is older than 7 days, rerun before merging and removed new_contributor Help guide this first time contributor stale_ci CI is older than 7 days, rerun before merging labels Apr 17, 2020
@gundalow
Copy link
Contributor

gundalow commented Jun 7, 2020

Are you still interested in setting up a dedicated collection for this?

@Akasurde
Copy link
Member

Akasurde commented Jul 3, 2020

@gundalow I think this is being handled in alibaba/alibaba.alicloud#272. Thanks.

@Akasurde Akasurde closed this Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_plugin New plugin stale_ci CI is older than 7 days, rerun before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants