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

Name and Provider fields cleaning #1321

Merged
17 commits merged into from
Oct 19, 2018
Merged

Conversation

SYNchroACK
Copy link
Contributor

Solution for: #1314

@SYNchroACK SYNchroACK modified the milestone: 1.1.1 Sep 7, 2018
@codecov-io
Copy link

codecov-io commented Sep 11, 2018

Codecov Report

Merging #1321 into maintenance will increase coverage by <.01%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##           maintenance    #1321      +/-   ##
===============================================
+ Coverage        75.63%   75.64%   +<.01%     
===============================================
  Files              273      273              
  Lines            12868    12872       +4     
  Branches          1746     1748       +2     
===============================================
+ Hits              9733     9737       +4     
  Misses            2738     2738              
  Partials           397      397
Impacted Files Coverage Δ
intelmq/bin/intelmq_gen_docs.py 95.65% <100%> (+0.26%) ⬆️

@SYNchroACK SYNchroACK requested a review from a user September 12, 2018 14:51
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Thanks for approaching this issue! Looks good overall.

The parameters in intelmq/bot/BOTS is not sorted anymore

Thanks also for fixing the missing/wrong parameters in feeds and bots documentation for some bots.

Could you please also fix the name of the parameter in docs/Bots.md section "Common parameters"? It's still called feed there.

dest="runtime_file",
metavar="<filepath>",
required=False,
help='/tmp/runtime.conf')
Copy link

Choose a reason for hiding this comment

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

Proposal:

help="write runtime configuration to the given file (e.g. '/tmp/runtime.conf') instead of stdout")

Same with the pipeline configuration

print("Runtime configuration written to: %s" % arguments.runtime_file)
else:
print("\nRuntime configuration:\n")
print(json.dumps(runtime_config, indent=4))
Copy link

Choose a reason for hiding this comment

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

We also use these parameters in most places: sort_keys=True, separators=(',', ': ')

Sorting is necessary for a reproducible ordering.

@ghost
Copy link

ghost commented Sep 13, 2018

I fixed the bin/rewrite_config_files to do the BOTS sorting (more) correctly in ad5afcb
So you can use this file to fix it. I did not apply the scrip to the file yet to avoid conflicts.

@ghost ghost added this to the 1.1.1 milestone Sep 13, 2018
@ghost ghost added bug Indicates an unexpected problem or unintended behavior documentation Indicates a need for improvements or additions to documentation documentation: feeds About intelmq/etc/feeds.yaml labels Sep 13, 2018
@ghost ghost self-assigned this Sep 13, 2018
@ghost ghost merged commit 2888801 into certtools:maintenance Oct 19, 2018
ghost pushed a commit that referenced this pull request Oct 19, 2018
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior documentation: feeds About intelmq/etc/feeds.yaml documentation Indicates a need for improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants