-
-
Notifications
You must be signed in to change notification settings - Fork 107
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
feat: add slack ID of all members #617
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
cc : @KhudaDad414 @derberg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR!!!
added small comments regarding schema
"slack":{ | ||
"type":"string", | ||
"description": "It must be a valid slackID. It is the ID of the slack of the person 's profile", | ||
"examples":[ | ||
"U01QPAP1QWE" | ||
] | ||
|
||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"slack":{ | |
"type":"string", | |
"description": "It must be a valid slackID. It is the ID of the slack of the person 's profile", | |
"examples":[ | |
"U01QPAP1QWE" | |
] | |
}, | |
"slack": { | |
"type": "string", | |
"description": "It must be a valid slackID. It is the ID of the person in the AsyncAPI Slack workspace.", | |
"examples": [ | |
"U01QPAP1QWE" | |
] | |
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this field, to be useful, should be also added to line 79 to list of required fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@derberg should I add slackID there also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean slack
property should be mandatory in TSC members list. To enforce it, add it to line 79 in the schema, to the list of required props
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok ,doing the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sambhavgupta0705 we have a new TSC member added to list -> Barbaño González
please add her slack id too 🙏🏼
ok |
@derberg can I get the details of Barbaño González such as available for hire and repos she is maintaining. |
@Barbanio what repos do you maintain again? 🤔 Is it |
She works at Postman with us 😄 |
Hello @sambhavgupta0705! Yes, I'm a maintainer in the |
@sambhavgupta0705 but you did not have to add @Barbanio as she was there already, just slackID was missing 😄 Now when you added you have conflicts with what is in |
This was already in
|
@derberg can you help me I think other changes of master are also reflected here:( I did git pull and git merge master which did this menace:(( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry mate but the only solution I see:
- copy changes you did in TSC_MEMBERS.json to your pocket 😄
- close this PR
- open new PR with new branch, cut from latest master
- paste changes from the pocket 😄
I really do not have a better solution, sorry
ok doing it |
Description
Related issue(s)
resolves #606