You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by nickwallen <gi...@git.apache.org> on 2017/11/28 21:00:50 UTC
[GitHub] metron pull request #852: METRON-1239 Drop extra dev environments
GitHub user nickwallen opened a pull request:
https://github.com/apache/metron/pull/852
METRON-1239 Drop extra dev environments
Per the previous discussion referenced in JIRA, this PR drops support for the Quick Dev and Code Lab environments. These have been effectively dead for quite a long time.
I tested this by performing a Full Dev deployment and Amazon EC2 deployment just to make sure I didn't delete anything that I should not have.
I'd really like to get the metron-deployments directory cleaned-up and reorganized to make it easier for new users to build and deploy Metron. This is the first step.
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [ ] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
- [ ] Have you included steps or a guide to how the change may be verified and tested manually?
- [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
- [ ] Have you written or updated unit tests and or integration tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nickwallen/metron METRON-1239
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/852.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #852
----
commit 4a7484abd26a941754f18d0b498aedb2c24d385f
Author: Nick Allen <ni...@nickallen.org>
Date: 2017-11-28T19:44:58Z
METRON-1239 Drop extra dev environments
----
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/852
The api should be documented without requiring the installation of the product.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by anandsubbu <gi...@git.apache.org>.
Github user anandsubbu commented on the issue:
https://github.com/apache/metron/pull/852
+1 (non-binding) @nickwallen . This is a much needed fix since it is now straight-forward to anyone new and wanting to try Metron.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/852
> The issue is that we should use this type of operation, but do it as part of maven no?
It doesn't seem that the utility is useful enough to maintain anymore. It had value once, but not any longer.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/852
+1 by inspection
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by anandsubbu <gi...@git.apache.org>.
Github user anandsubbu commented on the issue:
https://github.com/apache/metron/pull/852
Hi @nickwallen , I found one more reference to quick dev - [here](https://github.com/nickwallen/metron/blob/7145b06fff9f4ecc8e63d9cdd56ed955a49e676d/metron-interface/metron-rest/src/test/resources/README.vm#L208).
Also, I could see that the contents within [site/current-book](https://github.com/nickwallen/metron/tree/7145b06fff9f4ecc8e63d9cdd56ed955a49e676d/site/current-book) needs an update, since it does not have the changes. Does `mvn site` need to be run in order to fix this?
---
[GitHub] metron pull request #852: METRON-1239 Drop extra dev environments
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metron/pull/852
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/852
no, i mean auto documentation of the rest stuff ( and stellar stuff ) from maven builds is a good idea™
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/852
Thanks @ottobackwards . I just pushed that fix, plus some other little things I found.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/852
The issue is that we should use this type of operation, but do it as part of maven no?
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/852
My preference would be to just remove the endpoint documentation from the REST README and point users to Swagger where it's provided in context and easier to read. I see no value in duplicating that documentation.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/852
+1 by inspection. Great job, @nickwallen
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/852
Looks like @merrimanr commented on the email thread, so I'll copy it here for posterity.
> I wrote the ReadMeUtils class a long time ago as a way to make documenting
the REST endpoints easier. The Controller class methods are annotated so
that endpoint documentation is displayed in Swagger but it is also
duplicated in the README. It seemed like a good idea at the time to
provide a utility to make this easier so that you only had to document in
one place. It was actually helpful (to me anyways) when we first
introduced a large number of REST endpoints and saved some tedious
copy/pasting.
>
> In hindsight, there was no way of enforcing that we use the utility along
with the `README.vm` template. People intuitively edit the README.md
instead and the template quickly became stale. Eventually I got tired of
keeping the template in sync so I stopped using it as well. This class can
(and should) be safely removed.
I'd say just dump the template and the utility in this PR, since you'd already either have to clean it up or wait for another PR anyway.
---
Re: [GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by Ryan Merriman <me...@gmail.com>.
I wrote the ReadMeUtils class a long time ago as a way to make documenting
the REST endpoints easier. The Controller class methods are annotated so
that endpoint documentation is displayed in Swagger but it is also
duplicated in the README. It seemed like a good idea at the time to
provide a utility to make this easier so that you only had to document in
one place. It was actually helpful (to me anyways) when we first
introduced a large number of REST endpoints and saved some tedious
copy/pasting.
In hindsight, there was no way of enforcing that we use the utility along
with the `README.vm` template. People intuitively edit the README.md
instead and the template quickly became stale. Eventually I got tired of
keeping the template in sync so I stopped using it as well. This class can
(and should) be safely removed.
On Wed, Nov 29, 2017 at 7:16 AM, justinleet <gi...@git.apache.org> wrote:
> Github user justinleet commented on the issue:
>
> https://github.com/apache/metron/pull/852
>
> Glancing briefly, it looks like `ReadMeUtils` uses it as a template
> for the metron-rest README.md. Just running the main in there overwrites
> the metron-rest README.md. Which seems very odd, given that `ReadMeUtils`
> is in the test package.
>
> There seems to be no documentation of this class, or its purpose, and
> I didn't dig enough into the code to figure it out. Even not knowing the
> details and assuming I'm not misreading what's happening, I don't like that
> there's an expectation of editing a `README.vm` file, then running a
> program to produce the final output `README.md`. `README.md` can vary
> independently of `README.vm`. And it already has.
>
> It's outside the scope of this ticket, but at minimum, that class
> needs to be moved out of test, it needs to be actually documented what the
> purpose of it is, the steps to use it, etc. Right now, though, unless
> someone comes up with a compelling reason not to, I'm in favor of killing
> it entirely. I don't ever see that being properly managed, even if it does
> have some utility built in.
>
>
> ---
>
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/852
Glancing briefly, it looks like `ReadMeUtils` uses it as a template for the metron-rest README.md. Just running the main in there overwrites the metron-rest README.md. Which seems very odd, given that `ReadMeUtils` is in the test package.
There seems to be no documentation of this class, or its purpose, and I didn't dig enough into the code to figure it out. Even not knowing the details and assuming I'm not misreading what's happening, I don't like that there's an expectation of editing a `README.vm` file, then running a program to produce the final output `README.md`. `README.md` can vary independently of `README.vm`. And it already has.
It's outside the scope of this ticket, but at minimum, that class needs to be moved out of test, it needs to be actually documented what the purpose of it is, the steps to use it, etc. Right now, though, unless someone comes up with a compelling reason not to, I'm in favor of killing it entirely. I don't ever see that being properly managed, even if it does have some utility built in.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/852
No problem @ottobackwards . I just deleted the ReadMeUtils and README.vm files in the latest commit. I assume that whatever approach we take for documenting, we want to get rid of these files. If anyone disagrees with the latest commit, just let me know.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/852
I think you missed the inventory/quick-dev-platform stuff
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/852
> @anandsubbu: I found one more reference to quick dev - here.
I was confused about that README.vm file. Any idea what that is for?
> @anandsubbu: Also, I could see that the contents within site/current-book needs an update, since it does not have the changes.
I left everything under site/current-book untouched because I thought we only change that on a release. But I could be wrong about that.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/852
i'm not saying keep *this* utility
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by anandsubbu <gi...@git.apache.org>.
Github user anandsubbu commented on the issue:
https://github.com/apache/metron/pull/852
> I was confused about that README.vm file. Any idea what that is for?
I am not entirely sure, @nickwallen . I could find a lot of similarities between this README.vm and the main Metron REST README.md though.
https://github.com/nickwallen/metron/blob/7145b06fff9f4ecc8e63d9cdd56ed955a49e676d/metron-interface/metron-rest/README.md
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/852
We can tackle the issue of documenting in a discuss thread. I'm not going to tackle that issue on this PR.
---
[GitHub] metron issue #852: METRON-1239 Drop extra dev environments
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/852
agreed, didn't meet to hijack.
my +1 stands
---