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

Issue#5 #11

Merged
merged 12 commits into from
May 9, 2021
Merged

Issue#5 #11

merged 12 commits into from
May 9, 2021

Conversation

abhijoshi2k
Copy link
Member

@abhijoshi2k abhijoshi2k commented May 9, 2021

Thank you for contributing! Please check the following things before submiting your PR:

Required:

If necessary:

  • I have updated the README and documentation.
  • I have updated the ChangeLog with the changes I have made.

Replace this line with a short description about the changes you have made in this PR e.g. Corrected spelling mistakes in docs


Note: we will close your PR without comment if you do not check the required boxes above and provide ALL requested information.

@welcome
Copy link

welcome bot commented May 9, 2021

Thanks for opening this pull request! Make sure you have read the contributing guidelines, assigned an respective issue (if applicable) to this PR 😇

@abhijoshi2k
Copy link
Member Author

@PuneetGopinath I still have to add other things. Will request review once it's done.

@PuneetGopinath
Copy link
Member

PuneetGopinath commented May 9, 2021

Yes, No probs
@Andre601 can also help in adding this to readme

@PuneetGopinath PuneetGopinath added the minor Will be added to next minor release label May 9, 2021
@PuneetGopinath PuneetGopinath linked an issue May 9, 2021 that may be closed by this pull request
1 task
@PuneetGopinath PuneetGopinath added the Type: enhancement New feature or request label May 9, 2021
@abhijoshi2k
Copy link
Member Author

@PuneetGopinath I've added the required customizations. But we need to check once by merging.
Once we have tested everything, we will add it to readme.

@abhijoshi2k abhijoshi2k marked this pull request as ready for review May 9, 2021 06:45
@PuneetGopinath
Copy link
Member

@PuneetGopinath I've added the required customizations. But we need to check once by merging.
Once we have tested everything, we will add it to readme.

Sure

@abhijoshi2k
Copy link
Member Author

abhijoshi2k commented May 9, 2021

@PuneetGopinath Please review and merge. Let me know if any changes are required.

@PuneetGopinath
Copy link
Member

PuneetGopinath commented May 9, 2021

Can you run build?

@abhijoshi2k
Copy link
Member Author

Can you run build?

Done

@PuneetGopinath
Copy link
Member

Now last we have to add it to readme and changelog

@PuneetGopinath
Copy link
Member

I will add to changelog, you can add to readme

@abhijoshi2k
Copy link
Member Author

Now last we have to add it to readme and changelog

We need to test on our own repos first.

@abhijoshi2k
Copy link
Member Author

Now last we have to add it to readme and changelog

We need to test on our own repos first.

For that we must merge. Should we?

@PuneetGopinath
Copy link
Member

Yes, we have to merge branch main
But how can we test without merging this pr?

@abhijoshi2k
Copy link
Member Author

Yes, we have to merge branch main
But how can we test without merging this pr?

We can change branch in our workflow. Currently it is main. We can change it to issue#5

@PuneetGopinath
Copy link
Member

PuneetGopinath commented May 9, 2021

Ohh yes

@PuneetGopinath
Copy link
Member

No need to release new version right?

@abhijoshi2k
Copy link
Member Author

No need to release new version right?

Nope. We will release after testing

@abhijoshi2k
Copy link
Member Author

I tried using issue#5 branch, but because of # it failed, So I used test branch (test branch also failed, should we change default ranch)

It worked for me.

@abhijoshi2k
Copy link
Member Author

@PuneetGopinath
Copy link
Member

I tried using issue#5 branch, but because of # it failed, So I used test branch (test branch also failed, should we change default ranch)

It worked for me.

I made I mistake, anyway if it works, we can update readme, pull main and merge this pr

@PuneetGopinath
Copy link
Member

Don't release after merging this pr, #4 has to be completed

@abhijoshi2k
Copy link
Member Author

Don't release after merging this pr, #4 has to be completed

Okay. I'll update readme and merge. Will wait for release till #4 is completed.

@PuneetGopinath PuneetGopinath merged commit befafc9 into main May 9, 2021
@welcome
Copy link

welcome bot commented May 9, 2021

Congrats on merging your first pull request🎉! We are proud of you!

@PuneetGopinath PuneetGopinath deleted the issue#5 branch May 9, 2021 07:51
@Andre601
Copy link
Member

Andre601 commented May 9, 2021

What about a {URL} option that would resolve into a URL?
Or do both {ID} and {REPO} resolve into linking to the issue/PR and Repo respectively? Kind of limiting if you ask me.

@abhijoshi2k
Copy link
Member Author

What about a {URL} option that would resolve into a URL?
Or do both {ID} and {REPO} resolve into linking to the issue/PR and Repo respectively? Kind of limiting if you ask me.

{ID} and {REPO} both resolve into inking to the issue/PR and Repo respectively.
So clicking on those will take you to respective issue/PR and repository.

I don't understand what would be the use of {URL}

@Andre601
Copy link
Member

Andre601 commented May 9, 2021

What about a {URL} option that would resolve into a URL?
Or do both {ID} and {REPO} resolve into linking to the issue/PR and Repo respectively? Kind of limiting if you ask me.

{ID} and {REPO} both resolve into inking to the issue/PR and Repo respectively.
So clicking on those will take you to respective issue/PR and repository.

I don't understand what would be the use of {URL}

Was shown in my issue.
Essentially something like [`{REPO}#{ID}`]({URL}) which f.e. resolved into Readme-Workflows/recent-activity#11 would be possible.

@abhijoshi2k

This comment has been minimized.

@Andre601
Copy link
Member

Andre601 commented May 9, 2021

@Andre601 Can you give me an example output having {URL}?

{URL} would essentially do what {ID} and {REPO} do right now, which is directly linking to the issue, comment, PR

@abhijoshi2k
Copy link
Member Author

abhijoshi2k commented May 9, 2021

@Andre601 That would be raw format. Such formats are difficult for users with less knowledge of markdown.
We can add it though. Then if the {URL} is mentioned, it'll be treated as raw markdown and workflow will just replace text as it is and add the issue/pr URL to it.
But if user formats f.e. COMMENTS_ACTIVITY in improper way, output will be some very weird string.
I don't know if I'm able to explain my concern properly.
I just want to say that it will be difficult for user to format this.

We can instead add 1 more parameter:
URL_FORMAT --> Will contain how {URL} will be replaced f.e. {REPO}{ID} will give same result as you earlier mentioned.

So user will only have to add {URL} and not [{REPO}#{ID}]({URL})

@Andre601
Copy link
Member

Andre601 commented May 9, 2021

We can instead add 1 more parameter:
URL_FORMAT --> Will contain how {URL} will be replaced f.e. {REPO}{ID} will give same result as you earlier mentioned.

So user will only have to add {URL} and not [{REPO}#{ID}]({URL})

That would be an idea.

@abhijoshi2k
Copy link
Member Author

We can instead add 1 more parameter:
URL_FORMAT --> Will contain how {URL} will be replaced f.e. {REPO}{ID} will give same result as you earlier mentioned.
So user will only have to add {URL} and not [{REPO}#{ID}]({URL})

That would be an idea.

Okay. I'll work on it.

@PuneetGopinath PuneetGopinath mentioned this pull request May 9, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Will be added to next minor release Type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Option to customize the different activity messages
3 participants