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

Mtp: Add Armenia WTD group info #2147

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Mtp: Add Armenia WTD group info #2147

wants to merge 14 commits into from

Conversation

rosewms
Copy link
Collaborator

@rosewms rosewms commented May 8, 2024

Adding the Armenia WTD group info to the Meetups page.

Note, might update the page title to "Meetups and communities" to better reflect the new platforms available.

Merge 2118 before this PR.


📚 Documentation preview 📚: https://writethedocs-www--2147.org.readthedocs.build/

@plaindocs
Copy link
Contributor

This won't work as is, Sphinx is looking for a non existant meetup key, and we'll have to add some logic to handle that.

@rosewms
Copy link
Collaborator Author

rosewms commented May 10, 2024

This won't work as is, Sphinx is looking for a non existant meetup key, and we'll have to add some logic to handle that.

Any suggestions? I'd love a secondary solution because some of the groups are also creating location specific Write the Docs LinkedIn pages that I think it would be good to include.

@plaindocs
Copy link
Contributor

Yeah, I'll poke at the code, won't happen overnight but shouldn't be super tricky

@plaindocs plaindocs requested a review from mxsasha May 20, 2024 08:49
@plaindocs
Copy link
Contributor

Hey @rosewms this is good to go! Merge at will, and lemme know if oyu have any questions?

@plaindocs
Copy link
Contributor

This is related to #2242 and attempts to remove the need for an actual meetup.com URL as well

Copy link
Contributor

@mxsasha mxsasha left a comment

Choose a reason for hiding this comment

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

Good idea to add a schema for validation!

docs/_ext/meetups.py Outdated Show resolved Hide resolved
city: str(required=False)
website: str(required=False)
twitter: str(required=False)
meetup: str(required=False) # I can't figure out how to validate must contain either meetup or meetup_alt
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think there is a way to do this in the schema

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it feels like there should be.

We could refactor to something like:

source:
   # one of
   meetupcom: example
   linkedin: example
   other: example

and then I believe we could something along these lines https://github.com/23andMe/Yamale?tab=readme-ov-file#lists

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I've gone a ahead and done that, and it seems to work, thought it still breaks my brain a little.

Copy link
Contributor

@plaindocs plaindocs left a comment

Choose a reason for hiding this comment

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

This is mostly my PR now! But I'm happy with it :-p

link: str(required=False, none=False)

source_meetup:
meetup: str(required=True, none=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could call this and the other one "url"? So that you only flip whether url is under source_meetup or source_other. Feels cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I actually went the other way to

source:
  meetup:

and

source:
   other:

but I'm falling into the trap of half assing things:

#2242 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants