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

Make github a multisource widget #450

Merged
merged 4 commits into from
May 15, 2019

Conversation

Seanstoppable
Copy link
Collaborator

@Seanstoppable Seanstoppable commented May 13, 2019

Note, that this is a backwards incompatible change

Previous config:

repositories:
  wtf: "wtfutil"

New Config:

repositories:
- "wtfutil/wtf"

Note, that this is a backwards incompatible change

Previous config:

```
repositories:
  wtf: "wtfutil"
```

New Config:

```
repositories:
- "wtfutil/wtf"
```
@senorprogrammer
Copy link
Collaborator

It doesn't have to be backwards incompatible. There's already an established precedent for changing config types. See how opsgenie handles schedule.

In this case you'd first try to cast to UList. If that works then no change necessary. If that raises an error, then try to cast to UMap. If that doesn't raise an error, concat all the map's values and keys together (ie: "wtf": "wtfutil" becomes "wtfutil/wtf") and create the list of those.

@Seanstoppable
Copy link
Collaborator Author

Thanks! Will take a look

@Seanstoppable
Copy link
Collaborator Author

There is one major difference here in that MultiSourceWidget expects a specific config type to get the sources.
However, like how scrollable items were being handled, it was really only trying to use them to get the length, and I can refactor multisourcewidget to update that length vs reading config

@senorprogrammer
Copy link
Collaborator

I'm not sure what you mean. It looks like Multisource reads the len() of its Sources attribute, which is []string. That can still be built that way during the settings loading. Or am I missing something?

Parse repositories both ways
Have github manually set sources, since historical config may not handle
@Seanstoppable
Copy link
Collaborator Author

You are correct, I am now handling both configs and works fine by setting Sources manually in construction.

@senorprogrammer senorprogrammer merged commit 7c1152a into wtfutil:master May 15, 2019
@senorprogrammer
Copy link
Collaborator

Thanks!

@Seanstoppable Seanstoppable deleted the githubmultisource branch May 25, 2019 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants