-
Notifications
You must be signed in to change notification settings - Fork 153
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
[#2161] One-Stop Config File for Code Portfolio #2172
Conversation
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.
Added some comments about the file format.
authorGithubId: Sample Author Github ID | ||
branches: | ||
- name: Branch 1 | ||
blurb: Blurb 1 |
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.
Blurb should allow a few paragraph of markdown text. So, yml might not be the right format for this config file.
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.
@@ -0,0 +1,11 @@ | |||
repoUrl: github.com/user/repo | |||
reportTitle: RepoSense Report | |||
authorDisplayName: Sample Author |
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.
Not needed
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.
@damithc Sure thing, will remove these from the YAML file and other associated classes in a future commit.
- name: Branch 2 | ||
blurb: Blurb 2 | ||
startDate: 2020-01-01 | ||
endDate: 9999-12-31 |
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.
See #2161 (comment) for an example that seems intuitive from the user POV
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.
@damithc Just wanted to clarify, do you mean that we should include the configs within the markdown files itself, and have the backend parse the markdown files and then generate the reports, or that the overall order/style/keys in the YAML file should match what was used in the example provided?
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.
@asdfghjkxd I wasn't specifically using markdown either. It is just a custom format that makes sense from the user's point of view. I'm fine if something close can be achieved by YAML or some other commonly-used format, provided it makes sense from the user (i.e., the user shouldn't have to do a lot of extra work just to conform to the file format)
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.
@damithc I see, sure thing, we will try to design a user-friendly and convenient config file format and update this PR and parent issue with any new findings/improvements!
@asdfghjkxd Let's settle the file format first. It's best to design the user view of the feature before we start implementing. Giving a full-fledged example config file is a good way to see the user POV of this feature. |
Do note that this PR should not be merged until we have successfully migrated our CI/CD from Java 8 to Java 11. This PR requires #2183. In the meantime, CI will always fail since the JDK versions are conflicting. We can continue to work on this PR in the meantime, taking note of the failed CI tests over here. |
* [#2120] Update RepoSense contributors in documentation (#2138) The current About page on the RepoSense docs does not reflect the updated list of developers working on RepoSense. Let's move to update the list to more accurately reference the current developers of RepoSense. * [#2001] Extract c-zoom-commit-message component from views/c-zoom (#2132) Many of the frontend files are difficult to navigate through due to the large file sizes. As we add more features to the frontend, it is getting harder to maintain. This is also very unfriendly towards new contributors. Let us break down frontend files in a logical manner, continuing with extracting c-zoom-commit-message from views/c-zoom. * [#2142] Fix Vulnerabilities (#2143) Fix vulnerabilities in codebase. There are existing vulnerabilities in the codebase. Let's fix as many as possible. * Bump follow-redirects from 1.15.4 to 1.15.6 in /frontend (#2160) Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.4 to 1.15.6. - [Release notes](https://github.com/follow-redirects/follow-redirects/releases) - [Commits](follow-redirects/follow-redirects@v1.15.4...v1.15.6) * Bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /frontend (#2168) Bumps [webpack-dev-middleware](https://github.com/webpack/webpack-dev-middleware) from 5.3.3 to 5.3.4. - [Release notes](https://github.com/webpack/webpack-dev-middleware/releases) - [Changelog](https://github.com/webpack/webpack-dev-middleware/blob/v5.3.4/CHANGELOG.md) - [Commits](webpack/webpack-dev-middleware@v5.3.3...v5.3.4) * [#2151] Update LoadingOverlay and Minor Versions of Node Dependencies (#2152) Update LoadingOverlay and Minor Versions of Node Dependencies Some dependencies are not at their latest minor or patch releases. Let's update these dependencies, as well as LoadingOverlay as part of a bug fix. * Factor out markdown parser * [#2109] Add search by tag functionality (#2167) Add search by tag functionality It can be useful to search author/repos by git tags. Let's add this functionality to make it easier to filter by tags. This commit also fixes a bug that existed in a previous version of the feature which resulted in all users being shown to belong to same group. * Refactor chunks * Fix style * Add parts of blurb * Fix linting * Fix style * Fix missing html parsing * Remove unused import --------- Co-authored-by: George Tay <george_tay@u.nus.edu> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: jonasongg <120372506+jonasongg@users.noreply.github.com>
The following links are for previewing this pull request:
|
Part of #2161
Part of #2170
Proposed commit message
Other information
The code written for this feature is stored on the main repository to allow all team members to work on this concurrently.
Additionally, the following PRs/Tasks need to be completed before we can merge this PR: