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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate access to wgMessagesDirs #5147

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hexmode
Copy link
Member

@hexmode hexmode commented Nov 26, 2021

No description provided.

@JeroenDeDauw
Copy link
Member

LocalMessageProvider does not seem like a good place to put this. Is an entirely different responsibility by the looks of it. Can't say more without further investigation in IDE.

@hexmode
Copy link
Member Author

hexmode commented Nov 26, 2021

LocalMessageProvider does not seem like a good place to put this. Is an entirely different responsibility by the looks of it.

Should I just use MW's context object?

@JeroenDeDauw
Copy link
Member

My comment was not about using static fields and functions VS injecting something. Rather that the new static stuff gets glued onto LocalMessageProvider in this PR, which seems like an odd location.

(I do not really understand what you have in mind with context object)

@hexmode
Copy link
Member Author

hexmode commented Nov 27, 2021

(I do not really understand what you have in mind with context object)

I was thinking about how to get the value of wgMessagesDirs -- but I meant Config, not context.

But looking at that, I prefer the current implementation. If you do have a better place to put it, let me know. I may try again later, otherwise.

@JeroenDeDauw
Copy link
Member

A new "class" with the static stuff would be better than sticking it onto LocalMessageProvider.

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

Successfully merging this pull request may close these issues.

None yet

2 participants