Skip to content

Conversation

DevSerendipity
Copy link
Contributor

This is a pull request on resolving the issue#275
https://github.com/Together-Java/TJ-Bot/issues/275

The issue was resolved with adding a parameters to MethodUtils which would provide the tags into the footer for the embeds, also edited the values of 5 classes so they match the parameters for MethodUtils

@DevSerendipity DevSerendipity requested review from a team as code owners December 7, 2021 16:34
@Zabuzard
Copy link
Member

Zabuzard commented Dec 7, 2021

Could you add a screenshot on how it looks like please? Thanks.

@Zabuzard Zabuzard linked an issue Dec 7, 2021 that may be closed by this pull request
@Zabuzard Zabuzard added enhance command Modify or improve an existing command or group of commands of the bot priority: normal labels Dec 7, 2021
@Zabuzard Zabuzard added this to the Improvement phase 1 milestone Dec 7, 2021
public static @NotNull MessageEmbed generateEmbed(@Nullable String title,
@NotNull CharSequence content, @Nullable User user, @Nullable Color ambientColor) {
@NotNull CharSequence content, @Nullable User user, @Nullable Color ambientColor,
@Nullable String footerTag) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is overly specific. Change the role of footerTag to just a footer. And then move the user.getName() + footerTag logic into the call-site.

So that all callers can now use this parameter as footer and not as "footerTag".

@Zabuzard
Copy link
Member

Zabuzard commented Dec 7, 2021

Wait stop. This commit history is totally broken. Why does it contain the code from your other PR (#296 ), remove that stuff. You have to develop your features individually, isolated from each other.

@Zabuzard Zabuzard marked this pull request as draft December 7, 2021 17:29
@Zabuzard
Copy link
Member

Zabuzard commented Dec 7, 2021

Converted to draft to block merging before the git history is cleaned up (i.e. the dependency to #296 is removed).

@interacsion
Copy link
Contributor

interacsion commented Dec 7, 2021

Wait stop. This commit history is totally broken. Why does it contain the code from your other PR (#296 ), remove that stuff. You have to develop your features individually, isolated from each other.

@Zabuzard are you sure you've mentioned the right PR? cause #296 is my PR.

@Zabuzard
Copy link
Member

Zabuzard commented Dec 7, 2021

Wait stop. This commit history is totally broken. Why does it contain the code from your other PR (#296 ), remove that stuff. You have to develop your features individually, isolated from each other.

@Zabuzard are you sure you've mentioned the right PR? cause #296 is my PR.

Yes. Have a look at the commit history and the changes that this PR introduces. Basically, 8 of your 9 commits should not be here, they are from #296 and should not be in this PR (#308 ):

commits

@DevSerendipity
Copy link
Contributor Author

Wait stop. This commit history is totally broken. Why does it contain the code from your other PR (#296 ), remove that stuff. You have to develop your features individually, isolated from each other.

@Zabuzard are you sure you've mentioned the right PR? cause #296 is my PR.

Yes. Have a look at the commit history and the changes that this PR introduces. Basically, 8 of your 9 commits should not be here, they are from #296 and should not be in this PR (#308 ):

commits

well maybe you got it confused those are mine commits not @MaiTheLord , and also I have no idea how these commits entered here, giving that I have only created the branch and pushed the changes on it

@Zabuzard
Copy link
Member

Zabuzard commented Dec 7, 2021

well maybe you got it confused those are mine commits not @MaiTheLord , and also I have no idea how these commits entered here, giving that I have only created the branch and pushed the changes on it

You probably did something wrong when rebasing/pulling develop. In any case, those commits have to go.

@DevSerendipity DevSerendipity force-pushed the feature/replicating-slash-command branch from 7b13bce to 98d1611 Compare December 8, 2021 16:43
@Zabuzard
Copy link
Member

Closed in favor of #309

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhance command Modify or improve an existing command or group of commands of the bot priority: normal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slash Commands are not replicable
3 participants