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

#38: Refactor the Manifests class to an immutable #43

Merged
merged 1 commit into from
Jan 30, 2018
Merged

#38: Refactor the Manifests class to an immutable #43

merged 1 commit into from
Jan 30, 2018

Conversation

proshin-roman
Copy link
Contributor

I've made Manifests file immutable. As well I added asMap() method that returns a copy of an internal map of attributes. Method keySet() returns a copy of keys set too.

@0crat
Copy link

0crat commented Jan 17, 2018

@yegor256/z please, pay attention to this pull request

@0crat
Copy link

0crat commented Jan 17, 2018

Job #43 is now in scope, role is REV

@@ -40,15 +41,67 @@
* @since 1.1
* @see Manifests
*/
public interface MfMap extends Map<String, String> {
public interface MfMap {
Copy link
Member

Choose a reason for hiding this comment

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

@proshin-roman Maybe it's not a good idea to change the public interface of classes that are already in production? Also, what do we gain from this?

@yegor256
Copy link
Member

@proshin-roman see my comment. Also, what was the problem you were solving? What was wrong?

@proshin-roman
Copy link
Contributor Author

@yegor256 that's strange that the ticket is not mentioned there automatically. This pull request has been created for the ticket #38 and it meets the requirements of the ticket.

@yegor256
Copy link
Member

@rultor merge

@rultor
Copy link
Contributor

rultor commented Jan 30, 2018

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 76c102b into jcabi:master Jan 30, 2018
@0crat
Copy link

0crat commented Jan 30, 2018

Job gh:jcabi/jcabi-manifests#43 is not assigned, can't get start time

@rultor
Copy link
Contributor

rultor commented Jan 30, 2018

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 8min)

@0crat
Copy link

0crat commented Jan 30, 2018

The job #43 is now out of scope

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

Successfully merging this pull request may close these issues.

4 participants