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

Checkstyle: update addresses of DTD files #586

Merged
merged 6 commits into from Mar 16, 2019
Merged

Checkstyle: update addresses of DTD files #586

merged 6 commits into from Mar 16, 2019

Conversation

fzdy1914
Copy link
Member

@fzdy1914 fzdy1914 commented Mar 6, 2019

We are using the DTD files from http://checkstyle.sourceforge.net/
and http://puppycrawl.com/ in our config file of checkstyle.

However, due to security reason, checkstyle decided to remove DTDs from
above websites and ask users to use the DTD files from
https://checkstyle.org/[1].

Let's update the addresses of DTD files correspondingly.

As later version of the suppression DTD file has been released [2],
let's also update the DTD file to the latest version.

[1] Checkstyle Decide to Remove Some DTDs
checkstyle/checkstyle#6478
[2] Checkstyle Suppressions.xml Example
https://checkstyle.org/config_filters.html#SuppressionFilter_Examples

@fzdy1914 fzdy1914 changed the title edit the address of DTD file used for checkstyle edit the address of DTD files used for checkstyle Mar 6, 2019
@fzdy1914 fzdy1914 requested a review from a team March 6, 2019 15:28
@fzdy1914 fzdy1914 changed the title edit the address of DTD files used for checkstyle Edit the address of DTD files used for checkstyle Mar 6, 2019
@fzdy1914 fzdy1914 changed the title Edit the address of DTD files used for checkstyle Edit the addresses of DTD files used for checkstyle Mar 6, 2019
Copy link
Contributor

@emer7 emer7 left a comment

Choose a reason for hiding this comment

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

LGTM

For the commit message, if not mistaken avoid using the word currently as it is implicitly so

@yamidark
Copy link
Contributor

@fzdy1914

From https://checkstyle.org/config_filters.html#SuppressionFilter_Examples, I don't seem to find anything that mentions we should be using version 1.2 though (it just gives an example usage)? Did you give the wrong link, or is there anywhere specific I should look at?

Also, do give a short summary of the link you gave, example in this commit.

@fzdy1914
Copy link
Member Author

@fzdy1914

From https://checkstyle.org/config_filters.html#SuppressionFilter_Examples, I don't seem to find anything that mentions we should be using version 1.2 though (it just gives an example usage)? Did you give the wrong link, or is there anywhere specific I should look at?

Also, do give a short summary of the link you gave, example in this commit.

The example is what I am referring to. Because it is the only place that checkstyle mention about the version of its suppression.xml. Since it is an example that gives instructions on how to write this kind of file, so I interpreted it as the suggested version. Maybe you can suggest a way to rephrase it?

@yamidark
Copy link
Contributor

Since it is an example that gives instructions on how to write this kind of file, so I interpreted it as the suggested version. Maybe you can suggest a way to rephrase it?

If this is the latest version available, can just mention that instead.

@fzdy1914 fzdy1914 requested a review from a team March 15, 2019 09:33
@yamidark
Copy link
Contributor

Also, do give a short one line summary of any link you gave, example in this commit.

Remember to add this in as well.

Copy link
Member

@eugenepeh eugenepeh left a comment

Choose a reason for hiding this comment

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

Nits to the last paragraph:

Meanwhile, update the version of suppression DTD file to 1.2 because it is
the latest version mentioned. [2]

to

As later version of the suppression DTD file has been released [2],
let's also update the DTD file to latest version.

Copy link
Contributor

@yamidark yamidark left a comment

Choose a reason for hiding this comment

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

Do update the message according to @eugenepeh comment above

@yamidark yamidark changed the title Edit the addresses of DTD files used for checkstyle Checkstyle: update addresses of DTD files Mar 16, 2019
@yamidark yamidark merged commit 629e96c into reposense:master Mar 16, 2019
@fzdy1914 fzdy1914 deleted the checkstyle branch March 16, 2019 10:57
ongspxm added a commit that referenced this pull request Mar 19, 2019
The way `v_summary` handles the loading of the sidebar
assumes there is only one type of sidebar view.

In light of the new "zoomin" view, we would have to extend and
generalize the way we load the sidebar, so as to be able to load both
the authorship and zoomin view using the same mechanism.

The design of the "event bus" is as such. The actual loading of the
tabs is done in the main app, and those functions usually have the form
`updateTabXXX()`, which write the corresponding information object into
`this.tabInfo` and does `this.tabActive='XXX';`. This will then load
the right tab in the sidebar.

To load the sidebar from without another component, the VueJS event
emitting mechanism is used. A function of the form  `openTabXXX()` is
used to call the component's `$emit()`. The main application then
handles the emitted message using the corresponding `updateTabXXX()`.

* Add highlighting to ramps (#544)

To be able to open up a "zoomed-in" view of the ramp, the user must
first be able to select part of the ramp to focus on.

Let's add a way for the user to highlight the range of the ramp to
focus on. This will later translate into the date range for the
"zoomed-in" view to display the relevant commits.

In this particular implementation, we use a global `drags` object as a
way of preventing user from highlighting on multiple ramp charts. E.g.
if a user mousedown on one ramp chart and mouseup on the other, the
second chart will be highlighted with the positions defined between the
two mouse events.

* Merge 'tabs' to get refactored v_authorship

commit b632566
Author: ongspxm <ongspxm@gmail.com>
Date:   Mon Feb 25 00:46:12 2019 +0800

    added TODO to remove change

commit 4bb21d6
Author: ongspxm <ongspxm@gmail.com>
Date:   Fri Feb 22 23:38:34 2019 +0800

    updated to use isTabActive instead of tabActive to trigger tab display

commit a7aeefd
Author: ongspxm <ongspxm@gmail.com>
Date:   Thu Feb 21 22:39:18 2019 +0800

    fix lint

commit b813aa2
Author: ongspxm <ongspxm@gmail.com>
Date:   Thu Feb 21 22:30:35 2019 +0800

    wrap everything within v-authorship

commit 7dbb41b
Author: ongspxm <ongspxm@gmail.com>
Date:   Thu Feb 21 22:25:45 2019 +0800

    updateCount using document.getElementsByClassName

commit c66a3ee
Author: ongspxm <ongspxm@gmail.com>
Date:   Thu Feb 21 15:53:22 2019 +0800

    wip. left with updateCount

commit 9e73331
Author: ongspxm <ongspxm@gmail.com>
Date:   Thu Feb 21 15:49:48 2019 +0800

    move expand func to v_authorship

commit 3fcd62b
Author: ongspxm <ongspxm@gmail.com>
Date:   Thu Feb 21 15:43:50 2019 +0800

    remove js for button update

commit 4fc1151
Author: ongspxm <ongspxm@gmail.com>
Date:   Thu Feb 21 15:37:02 2019 +0800

    deactivating the tab

commit a387e19
Author: ongspxm <ongspxm@gmail.com>
Date:   Thu Feb 21 14:51:58 2019 +0800

    wip

* move call emitting to component func

* handle opening of tabs in main app

* refactoring ramps to their own component (#572)

Currently, the entire ramp template resides in `v_summary`. In light of
the "zoomin" tab view, we would have to reuse ramp chart view to
display the commits in the "zoomin" tab.

Let's refactor to move the ramp into its own component, so as to
support reuse in the "zoomin" tab view.

* fix lint

* fix: show min max date on authorship view

* lint

* [#554] Rename 'dashboard' instances to 'report' (#579)

The term 'report' and 'dashboard' are used throughout RepoSense to
refer to the same thing, the result after running analysis.

However, users may not understand the subtle differences between
these two terms. Instead, using a single term would help in
standardization and comprehensibility.

Let's rename all usage of 'dashboard' instances to the term 'report'
instead, as standardized in #220.

* Checkstyle: update addresses of DTD files #586

We are using the DTD files from http://checkstyle.sourceforge.net/
and http://puppycrawl.com/ in our config file of checkstyle.

However, due to security reason, checkstyle decided to remove DTDs from
above websites and ask users to use the DTD files from
https://checkstyle.org/[1].

Let's update the addresses of DTD files correspondingly.

As later version of the suppression DTD file has been released[2],
let's also update the DTD file to the latest version.

[1] Checkstyle removes DTDs from http://checkstyle.sourceforge.net:
checkstyle/checkstyle#6478

[2] Checkstyle Suppressions.xml example:
https://checkstyle.org/config_filters.html#SuppressionFilter_Examples

* [#540] Perform cloning in parallel with analyzing (#560)

Repos are processed sequentially, one at a time.

As cloning does not takes much processing power and analysis does not
take any network bandwidth, the two can be done in parallel to reduce
the total processing time.

Here are some test results to support the above hypothesis:
The test was performed using CS2103 AY1819S1 project repos on my local
machine. The average time taken to generate the report was measured
across 10 runs. Current code took 17 min 21s while new implementation
took 11 min 55s.

Let's clone the next repo in the list while the current repo is being
analyzed.

* [#465] Url: bookmark opened code view (#524)

Our report's url changes along with the state of the dashboard as a
mechanism to allow users to easily revisit her last view as well as
easily be shared with other people.

However, this was only limited to the chart view configuration, nothing
of the code view were part of the state being saved.

To allow users to easily restore their last reviewed code view, let's
also encode any opened code view into the url.

* [#510] Remove unused Checkstyle analysis feature (#597)

The checkstyle analysis feature was added to the code base in the
early stage of RepoSense, when there was some hope of running static
analysis over the code written by authors of the repositories.

As this feature has not been used since early stages(v1.0), and it
is also not in our immediate plans anymore, leaving it will
unnecessarily complicate the codebase.

Let's clean up the code base by removing this unused checkstyle
analysis feature.

* fix: codeview not opening

* update view opeing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants