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
Contributing Info UI #467
Contributing Info UI #467
Conversation
Awesome! Some suggestions:
|
@@ -30,6 +37,18 @@ case class ProjectForm( | |||
) | |||
} | |||
|
|||
val oldBeginnerIssueLabel = project.github.flatMap(_.beginnerIssuesLabel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in the Github Step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put it there so that when the user sets the beginner issues label in the edit project page, this code will be run so that it can grab the beginner issues right away.
If this code was in the Guthub step, the beginner issues wouldn't get downloaded until the next time scaladex is re-indexed so the user would have to wait until then for their project to show up in the front page panel, contributing search results, ...
The next thing I want to add is to get the beginner issues in the client when the user sets the beginner issues label in the edit project page (similar to how you added updating the readme in the client), so that the user can then select which specific issues will be displayed for their project. This will get rid of the above code completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is better here rather than in indexing.
@@ -172,6 +172,42 @@ main { | |||
@include box-shadow(0 1px 15px rgba(0, 0, 0, 0.15)); | |||
} | |||
} | |||
|
|||
.contributing-projects { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some screenshot with a smaller window?
class DataRepository( | ||
github: Github, | ||
paths: DataPaths, | ||
githubDownload: GithubDownload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ibid as the previous comment. Github downloads should happen when indexing.
search(indexName / projectsCollection) | ||
.query( | ||
functionScoreQuery(contributingQuery) | ||
.scorers(randomScore(scala.util.Random.nextInt(10000))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be good to double check if this means a + b + c + random
where a,b, c
existsQuery("github.beginnerIssues")
existsQuery("github.contributingGuide")
existsQuery("github.chatroom")
are a, b, c values between 0 and 1? This would make 1 + 1 + 1 + large random value.
} | ||
|
||
@for(contributingGuide <- github.contributingGuide; chatroom <- github.chatroom) { | ||
<table> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try to avoid tables, they are harder to style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MichaelViveros and I will deal with that together 🙂
@MasseGuillaume Thanks
Where should this button go? To the left of the "Sorting" label? Alternatively, I could add a link to the normal search in the description for "Contributing Search" on the contributing search page (like how I had link to the Contributing Search on the front page).
What do you mean by "above the fold"? Where the Scaladex and Github Login buttons are? And this button would jump to the "Looking for a project to contribute to?" section of the front page? |
This is really great! On the first pass, @MichaelViveros and I will have to go through the template changes together. For now, let's not worry so much about that. (I will help). But right now in review, let's focus on changes to everything else other than the views. Generally everything looks good to me. I have questions about a few refactorings... |
Why not have small text beneath the "All the projects below..." text that says "go to normal search" or something? |
Good idea. Let's think about how we can do this so it's not too distracting. |
I would say on the left because it's a filter. Maybe above the filter label?
Above the fold is the part of the screen when you load a page that you don't have to scroll. On my screen it's 900px. It could be on top of the keyword list for example. |
@heathermiller Will do
@MasseGuillaume Got it, thanks for the clarification |
Feedback from our meeting:
|
Could you elaborate on this? What will get run ( |
via the cron command. I need to ask the sysadmin for this daily it would run ./index.sh |
@MasseGuillaume @heathermiller For the link to all of the beginner-friendly issues, which is better?
|
@MichaelViveros Option #2 is better, assuming that the list is populated with at least 3 issues. |
@heathermiller @MasseGuillaume In addition to a button, I was thinking of adding a filter (called Have Contributing Info) so that you could see how many/which projects in the current search have contributing info (Ex. 5 in the screenshot below). So there would be a Contributing Search button above the filters that takes the user to the Contributing Search page (where results will show contributing info) and there is also a Have Contributing Info filter (where results are the same as the normal search and show project description, targets, scala versions). Does this sound like a good idea or is just the Contributing Search button good enough (leave out the Have Contributing Info filter)? |
Sure, this is a good idea. I'd not put it at the top though. Probably better at the bottom of the list of possible filters. Also, maybe call it something else? E.g., "Contributors wanted" or something? What about also taking into consideration the "contributors wanted" badge as well? Is there some way unify these things? |
The list is pretty long, however.
👍 |
I think I'll leave out the Have Contributing Info filter actually (the Contributing Search button should be good enough, there's already a lot of filters like Gui mentioned).
@heathermiller Yeah, I think having the contributing info/panel and Contributors Wanted badge separate at the moment is confusing. Perhaps the I think it makes the badge more meaningful - with the Contributors Wanted badge all the maintainer had to do was set Contributors Wanted to true in the edit project page but with the Contributor-Friendly badge the maintainer has to make sure their project has beginner-friendly issues, contributing guide, chatroom. But this might make projects that don't have those 3 things look "unfriendly" which we don't want to do. Perhaps there's a better name for the badge than Contributing-Friendly. |
Got some of the minor tasks done (updated checkboxes). Here's a screenshot of the Contributing Search page with the button to switch between the normal search and contributing search, a link to more beginner friendly issues and link to a project's Code of Conduct (COC). The normal search page has a similar button to go to the Contributing Search page. @MasseGuillaume I fixed #473 for the Contributing Search so that selecting a filter and then typing a search term will narrow down the results. I fixed it by adding a minimum score to the function score query that is generated when searching elastic search ( Let me know what you think of this solution. Ex. Select filter for Js Target, 4 results: Then enter "quill" as query string, 1 result: |
Looking Good! Is this PR ready to merge. Anything more we should add? Make shure to run Scalafmt: https://travis-ci.org/scalacenter/scaladex/builds/258340312#L522 |
My bad about not running scalafmt. It's missing the features below but they're minor and I can make another PR for them. The first one about adding more projects with contributing info definitely needs to be done before the the server is re-indexed. If you just want to merge this PR (and not deploy it), then go ahead. If you want to deploy it and re-index, I can add some more projects over the next hour (by making a commit to projects.json in scaladex-index).
|
@MasseGuillaume Submitted a PR to scaladex-index for adding more repos - scalacenter/scaladex-index#1 This should be good to go once that's resolved |
This should not fail: https://travis-ci.org/scalacenter/scaladex/builds/258938217#L864 You only need the GitHub credential to run the GitHub task. |
Don't think that error is due to my PR. Yes, my PR added a dependency on
No, the |
@heathermiller Action items from call:
|
Search based on issue title Add new feature banner Select issues in edit page Store all beginner-friendly issues (not just selected ones) Add code of conduct to edit page
9f9d9cd
to
5bcecb5
Compare
Rebased and merged to master. Create another PR for further improvements. |
@heathermiller @MasseGuillaume @julienrf
New features:
projects.json
and when the data is re-indexed, it will fetch and store issues with that label for the projectTODO:
I tried pushing this code to the dev server but got some memory errors when trying to run it, I'll let you know when those are resolved so you can see things for yourself.
Front Page:
Search Page:
Project Page: