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

Update settings.yml for OSPO Landscape #78

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

Conversation

anajsana
Copy link

@anajsana anajsana commented Apr 1, 2024

edit categories and group view

edit categories and group view

Signed-off-by: Ana Jimenez Santamaria <[email protected]>
@anajsana anajsana changed the title Update settings.yml for OSPO Lanscape Update settings.yml for OSPO Landscape Apr 1, 2024
Copy link
Collaborator

@tegioz tegioz left a comment

Choose a reason for hiding this comment

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

Thanks @anajsana!

I've left some notes to discuss 🙂

- name: Members and Associates
categories:
- TODO Group Member
- OSPO Associate
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like there isn't a category named OSPO Associate in the landscape.yml file yet. There is a subcategory named like that though, but this section is expecting categories.

Copy link
Author

Choose a reason for hiding this comment

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

I see! I was planning to first make changes here and latter change the landscape.yml, but I can first go to landscape.yml and make the changes before merging this PR 👍

- Enterprise
- Public Sector
- Nonprofit
- Academic
Copy link
Collaborator

Choose a reason for hiding this comment

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

OSPO Adоpter still appears as a subcategory and I cannot find the new ones you've added yet in the landscape.yml file. Is this something you're planning to update soon?

Copy link
Author

Choose a reason for hiding this comment

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

related to my comment above. I can first go to landscape.yml and make the changes before merging this PR 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds great!

Let's do something first to make sure this set of changes works fine. As you've seen in the settings file, we've overriding the OSPO Adopter category to specify the order in which we display its subcategories. The problem with this approach is that only the subcategories listed here will be displayed (others will be ignored). So when you add some more, they won't show up as it'll be annoying.

My suggestion here is to omit the categories section temporarily. This way, you can reorganize everything as you prefer in the landscape.yml file and everything will work fine. The only difference will be that the order in which the subcategories will be displayed may vary depending on the window size.

Once you are ready, you can open a PR to setup the groups using the new categories. In the same PR, you could add back the OSPO Adopter category to specify the order of the subcategories if you'd like (if this is important for you, of course, otherwise it's better to leave it dynamic).

How does this sound? 🙂

Copy link
Author

Choose a reason for hiding this comment

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

@tegioz I've created the PR in the landscape.yml could you please confirm the structure is aligned with the changes requested in this settings.yml file?

https://github.com/todogroup/ospolandscape/pull/214/files

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well if you have it already we may not need this 😇

Let me take a look 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you take a look at the validation result? It looks like there is some sort of indentation problem:

https://github.com/todogroup/ospolandscape/actions/runs/8509469370/job/23305055994?pr=214

I would strongly recommend to make that check required before a PR can be merged, to avoid breaking the landscape.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In addition to that, there isn't yet a category named OSPO Associate. This affects the groups defined below.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let me know if you need any help with this @anajsana 🙂

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.

2 participants