-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add the basic structure and some first rules for InnerSource repo lin… #924
Conversation
src/main/java/com/sap/oss/phosphor/fosstars/data/github/GithubInnerSourceTopicDataProvider.java
Show resolved
Hide resolved
src/main/java/com/sap/oss/phosphor/fosstars/data/github/GithubInnerSourceTopicDataProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/com/sap/oss/phosphor/fosstars/data/github/GithubInnerSourceTopicDataProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/com/sap/oss/phosphor/fosstars/model/feature/oss/OssFeatures.java
Outdated
Show resolved
Hide resolved
src/main/java/com/sap/oss/phosphor/fosstars/tool/format/CommonFormatter.java
Outdated
Show resolved
Hide resolved
Overall, the changes look good. I have left few comments on what you could use
Apologies for taking time to review this. Also, we can try in parallel to set this up, how OSS Rules-of-play generate reports |
src/main/java/com/sap/oss/phosphor/fosstars/tool/format/CommonFormatter.java
Outdated
Show resolved
Hide resolved
Issue: #931 |
@sourabhsparkala as I updated everything already: any updates to this? |
@ihrigb I will look into this. In the mean time, there seems to be checkstyle issues, could you please address them? Thanks |
Optional<String> topicOptional = fetcher.githubTopicsFor(project).stream().filter(topic -> { | ||
return ACCEPTED_TOPICS.contains(topic.toLowerCase(Locale.US)); | ||
}).findFirst(); | ||
|
||
if (topicOptional.isPresent()) { | ||
return HAS_INNERSOURCE_TOPIC.value(true); | ||
} | ||
return HAS_INNERSOURCE_TOPIC.value(false) | ||
.explain("The repository does not have an InnerSource topic."); |
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.
Optional<String> topicOptional = fetcher.githubTopicsFor(project).stream().filter(topic -> { | |
return ACCEPTED_TOPICS.contains(topic.toLowerCase(Locale.US)); | |
}).findFirst(); | |
if (topicOptional.isPresent()) { | |
return HAS_INNERSOURCE_TOPIC.value(true); | |
} | |
return HAS_INNERSOURCE_TOPIC.value(false) | |
.explain("The repository does not have an InnerSource topic."); | |
boolean hasAcceptedTopic = fetcher.githubTopicsFor(project) | |
.stream() | |
.anyMatch(topic -> ACCEPTED_TOPICS.contains(topic.toLowerCase(Locale.US))); | |
return HAS_INNERSOURCE_TOPIC.value(hasAcceptedTopic) | |
.explainIf(false, "The repository does not have an InnerSource topic."); |
import java.util.Locale; | ||
import java.util.Optional; | ||
import java.util.Set; | ||
|
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.
Please add JAVA Docs to each method and variable
https://github.com/SAP/fosstars-rating-core/blob/master/src/main/java/com/sap/oss/phosphor/fosstars/data/github/IsApache.java
import com.sap.oss.phosphor.fosstars.model.value.ScoreValue; | ||
import java.util.Objects; | ||
|
||
public class InnerSourceRulesOfPlayRating extends AbstractRating { |
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.
import java.util.Objects; | ||
import java.util.Set; | ||
import java.util.stream.Collectors; | ||
|
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.
Please add the FEATURES used for the score calculation as a JAVA Doc
As given here https://github.com/SAP/fosstars-rating-core/blob/master/src/main/java/com/sap/oss/phosphor/fosstars/model/score/oss/BanditScore.java#L15-L23
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.
The changes look good. A few Java Docs are missing. Please note the below-required changes
- I believe you need to add entries of Features and Scores in the associated
*.java
files in the folder https://github.com/SAP/fosstars-rating-core/tree/master/src/main/java/com/sap/oss/phosphor/fosstars/tool/format - Checkstyle checks failing in the GitHub Checks https://github.com/SAP/fosstars-rating-core/pull/924/checks
…ting.